-----------------------------------------------------------
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
> 
>

Reply via email to