> On Sept. 8, 2015, 5:04 p.m., Joshua Cohen wrote: > > src/main/resources/scheduler/assets/js/controllers.js, line 185 > > <https://reviews.apache.org/r/38169/diff/1/?file=1065078#file1065078line185> > > > > 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 || ...);
This was done only to handle 'resource' column gracefully. Going with your proposal as a cleaner approach. > On Sept. 8, 2015, 5:04 p.m., Joshua Cohen wrote: > > src/main/resources/scheduler/assets/js/controllers.js, lines 206-213 > > <https://reviews.apache.org/r/38169/diff/1/?file=1065078#file1065078line206> > > > > 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? Makes sense. Done. > On Sept. 8, 2015, 5:04 p.m., Joshua Cohen wrote: > > src/main/resources/scheduler/assets/role.html, lines 36-37 > > <https://reviews.apache.org/r/38169/diff/1/?file=1065079#file1065079line36> > > > > 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. My only concern was referencing CSS components from the controller but I don't have much of opinion here. Having a controller-driver decision definitely feels cleaner than angular expression. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38169/#review98014 ----------------------------------------------------------- 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 > >
