Dashboard

PS didn't look through this – assuming its all broken up in the other ones

PS didn't look through this – assuming its all broken up in the other ones

I'd love to see the button/popover not even created if there are no registered elements – but this is fine

I'd love to see the button/popover not even created if there are no registered elements – but this is fine

I know this goes into AngularJS anti-patterns.... but .... form.controller('form').$$controls.forEach(...ect...) ... which does get rid of the DOM-y ness

I know this goes into AngularJS anti-patterns....

but .... form.controller('form').$$controls.forEach(...ect...) ... which does get rid of the DOM-y ness

wow! Wanna tell me what element you were having layout issues with?

wow!

Wanna tell me what element you were having layout issues with?

Could we use openlmis-table-container here – so that if someone removed .toolbar these unit tests would fail?

Could we use openlmis-table-container here – so that if someone removed .toolbar these unit tests would fail?

This seems a little odd that there is a dependency on this piece of markup being within the table container... I'd almost prefer a button directive that is compiled into the openlmis-table-contain...

This seems a little odd that there is a dependency on this piece of markup being within the table container...

I'd almost prefer a button directive that is compiled into the openlmis-table-container code...

Add this to a template so it can be overridden

Add this to a template so it can be overridden

Move this to a template so it can be easily over ridden

Move this to a template so it can be easily over ridden

Does it make sense to add unit tests for other parts of the tear down? There was a handful of stuff in there???

Does it make sense to add unit tests for other parts of the tear down? There was a handful of stuff in there???

Overall, I'd love to see the DOM-y aspects of this code moved over to the directive – as I think it would make this controller's $onDestroy a little simpler (it kinda feels like an easy to forget d...

Overall, I'd love to see the DOM-y aspects of this code moved over to the directive – as I think it would make this controller's $onDestroy a little simpler (it kinda feels like an easy to forget detail) – I also feel like the logic would be simpler if this logic didn't have DOM manipulation in it – but its your work, and we got deadlines!

If this runs after the ng-model inputs are bound to the original form (element) how do we know that the inputs are invalid? I'm assuming the form controller is getting that information somehow

If this runs after the ng-model inputs are bound to the original form (element) how do we know that the inputs are invalid? I'm assuming the form controller is getting that information somehow

Is there a reason for prepending the forms? I generally feel that appending form elements is more expected

Is there a reason for prepending the forms? I generally feel that appending form elements is more expected

Same thing twice?

Same thing twice?

So I've noticed this code everywhere. I think its ok to not make this code DRY. Its meaningful configuration.

So I've noticed this code everywhere. I think its ok to not make this code DRY. Its meaningful configuration.

Wow, this is a really low priority – could it be changed to 100 or something? This would help prevent someone from adding a compile function first.

Wow, this is a really low priority – could it be changed to 100 or something?

This would help prevent someone from adding a compile function first.

Isn't the naming convention openlmis-table-filter.directive:openlmisTableContainer

Isn't the naming convention openlmis-table-filter.directive:openlmisTableContainer

Pretty straight forward, I'll admit I'm probably not looking through these reviews in the right order - or this review is scoped too small to be meaningful

Pretty straight forward, I'll admit I'm probably not looking through these reviews in the right order - or this review is scoped too small to be meaningful

This file is confusing because it is not clear where 'openlmisTableFilters' is coming from, and since the difference between the two directives is an 's' its a little hard to read. I'm not sure if ...

This file is confusing because it is not clear where 'openlmisTableFilters' is coming from, and since the difference between the two directives is an 's' its a little hard to read. I'm not sure if there is a better way to name this parent/child relationship... but it would be great if there was.

Can this be moved closer to where its used?

Can this be moved closer to where its used?

Looks good – do you think this will remove fix that issue where we had to use ng-show/hide instead of ng-if???

Looks good – do you think this will remove fix that issue where we had to use ng-show/hide instead of ng-if???

WIP

OLMIS-2656: Fixed scss structure