----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38169/#review98014 -----------------------------------------------------------
src/main/resources/scheduler/assets/js/controllers.js (line 178) <https://reviews.apache.org/r/38169/#comment154161> A nit really, but I always feel like the safety of calling `hasOwnProperty` is overkill when we know ahead of time that there's no inheritance and we're not going to be using a name that will conflict with something higher up the prototype chain. The cleaner/simpler: var vector = consumption[column.map]; return !vector || (vector.numCpus > 0 || ...); src/main/resources/scheduler/assets/js/controllers.js (lines 198 - 205) <https://reviews.apache.org/r/38169/#comment154167> The repetition between the CPU, RAM, and Disk sections is enhanced by the addition of the additional types of consumption. Can you extract a function to reduce this? src/main/resources/scheduler/assets/role.html (lines 35 - 36) <https://reviews.apache.org/r/38169/#comment154160> I'd do this in the controller instead of the view. Add something like: $scope.resourcesClass = ...; Then in the view just do: <div class='{{resourceClass}}'> I don't *think* you need/want to use ng-class, since the value isn't going to change after render. - Joshua Cohen On Sept. 8, 2015, 3:14 a.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38169/ > ----------------------------------------------------------- > > (Updated Sept. 8, 2015, 3:14 a.m.) > > > Review request for Aurora, David McLaughlin and Joshua Cohen. > > > Bugs: AURORA-1461 > https://issues.apache.org/jira/browse/AURORA-1461 > > > Repository: aurora > > > Description > ------- > > Making Resources table dynamic to display only non-zero resource vectors (see > pics attached). Table width is now dependent on the number of visible > columns. > > Not particularly happy about ng-class approach to control table width, so any > suggestions how to make it better are welcome. > > May not apply cleanly, diffed against 38081. > > > Diffs > ----- > > src/main/resources/scheduler/assets/js/controllers.js > 98920196db34f2eb4dcad93429274517e7383efe > src/main/resources/scheduler/assets/role.html > 2572c4b4497aa78d3fa31457a5dc7c57e6565027 > > Diff: https://reviews.apache.org/r/38169/diff/ > > > Testing > ------- > > > File Attachments > ---------------- > > resources_full.png > > https://reviews.apache.org/media/uploaded/files/2015/09/08/ada720ce-bb97-4321-99bb-0574bf14b653__resources_full.png > resources_partial.png > > https://reviews.apache.org/media/uploaded/files/2015/09/08/a6e4aa7e-ca7e-4ca4-900e-fda6cfdcaff3__resources_partial.png > resources_min.png > > https://reviews.apache.org/media/uploaded/files/2015/09/08/c44964f2-d8d7-41b0-8a4f-995a1e3538f1__resources_min.png > > > Thanks, > > Maxim Khutornenko > >
