aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Suman Karumuri" <ma...@apache.org>
Subject Re: Review Request 21247: Add config grouping visualisation to job page
Date Sat, 10 May 2014 02:14:44 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21247/#review42626
-----------------------------------------------------------


I think we should show the config bar even when there is one config. It will also act a visual
indication to the user that all his tasks are in a consistent config.


File Attachment: Viewing multiple configs at the same time - Screen Shot 2014-05-08 at 4.25.42
PM.png
<https://reviews.apache.org//r/21247/#fcomment7>
    What happens when there are more than 2 configs? Will these configs wrap around?
    
    please add a close option to divs so the users can hide the configs they don't want. That
will let users compare the different configs all at once.


src/main/resources/org/apache/aurora/scheduler/http/ui/configSummary.html
<https://reviews.apache.org/r/21247/#comment76448>

    wrap this in a div.



src/main/resources/org/apache/aurora/scheduler/http/ui/groupSummary.html
<https://reviews.apache.org/r/21247/#comment76449>

    Please make this an abbr and add the label to the tooltip also. If there are too many
different configs or if the label is too long, the user would still be able to look at it.



src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js
<https://reviews.apache.org/r/21247/#comment76451>

    please only pass the required fields into the fields into the function. By passing in
$scope, it is very unclear what inputs buildGroupSummary depends on.



src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js
<https://reviews.apache.org/r/21247/#comment76453>

    please move all the code related to group summary into it's own controller. That will
make the code modular and easily testable.



src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js
<https://reviews.apache.org/r/21247/#comment76452>

    Please initialize all the new fields to scope outside this function. As is, a typo in
a variable name will cause the UI to render incorrectly and it would be really hard to debug
what went wrong.


- Suman Karumuri


On May 8, 2014, 11:45 p.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21247/
> -----------------------------------------------------------
> 
> (Updated May 8, 2014, 11:45 p.m.)
> 
> 
> Review request for Aurora, Suman Karumuri and Bill Farner.
> 
> 
> Bugs: AURORA-378
>     https://issues.apache.org/jira/browse/AURORA-378
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Replace the button to show/hide configs with a bar when there are multiple. Tidied up
the display of the configuration data. 
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 983101277ffbd1c4017b29f4c86e61315f1bcc78

>   src/main/resources/org/apache/aurora/scheduler/http/ui/configSummary.html PRE-CREATION

>   src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css dc515ac818a9af36522bb07a125cd92ff9230fa2

>   src/main/resources/org/apache/aurora/scheduler/http/ui/groupSummary.html PRE-CREATION

>   src/main/resources/org/apache/aurora/scheduler/http/ui/job.html cd78c20bee7891a6cdfd19943a6f7e8a9dce33df

>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 7c80bd769b7e80bef0822846166959925b1bf7f8

>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js a79276d2486fc1122a8b084ea0614cdae759f88c

> 
> Diff: https://reviews.apache.org/r/21247/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> Job page with only one config group
>   https://reviews.apache.org/media/uploaded/files/2014/05/08/908570f2-bef2-4370-bf08-1b5c7400c3c6__Screen_Shot_2014-05-08_at_4.26.02_PM.png
> Showing config for only one config group
>   https://reviews.apache.org/media/uploaded/files/2014/05/08/1424f2c3-cf66-4ce4-a595-dc3f7647d6d2__Screen_Shot_2014-05-08_at_4.26.12_PM.png
> Job page with multiple config groups
>   https://reviews.apache.org/media/uploaded/files/2014/05/08/bc9c1616-3c71-4bcc-ab40-fefc56d5e19a__Screen_Shot_2014-05-08_at_4.25.21_PM.png
> Viewing one of the groups from config bar
>   https://reviews.apache.org/media/uploaded/files/2014/05/08/6b8e6077-c8c8-48c9-94e9-732e782204b6__Screen_Shot_2014-05-08_at_4.25.36_PM.png
> Viewing multiple configs at the same time
>   https://reviews.apache.org/media/uploaded/files/2014/05/08/b7b2cba1-034b-43e6-9eaa-3ba1a78f87f5__Screen_Shot_2014-05-08_at_4.25.42_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message