facility-cce-status.html

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
But we use this flag in controller too, we don't want to have business logic both in controller and html. Plus it's tested. Plus it can change in future and it should be easily changeable in one pl...

But we use this flag in controller too, we don't want to have business logic both in controller and html. Plus it's tested. Plus it can change in future and it should be easily changeable in one place.

Chongsun Ahn I'm resolving this comment as I believe the popover issue has been fixed. Please, let us know if you think otherwise! https://review.openlmis.org/static/ogdo0b/2static/images/wiki/icon...

Chongsun Ahn I'm resolving this comment as I believe the popover issue has been fixed. Please, let us know if you think otherwise!

Paweł Albecki Do we need this flag? Couldn't we use vm.inventoryItems.length in html?

Paweł Albecki Do we need this flag? Couldn't we use vm.inventoryItems.length in html?

OLMIS-4475: monor improvements in facility cce status component

  1. … 2 more files in changeset.
This should be set inside the $onInit method.

This should be set inside the $onInit method.

Remove double dot

Remove double dot

OLMIS-4475: add missing quite mark

OLMIS-4475: disable popover when no cce

  1. … 2 more files in changeset.
No, I didn't fix popover issue yet

No, I didn't fix popover issue yet

yes, it's direct child

yes, it's direct child

Does this solve the popover issue? If there is no CCE at the facility, does the popover not show up anymore?

Does this solve the popover issue? If there is no CCE at the facility, does the popover not show up anymore?

I don't like that we're setting the alert status class outside the function that does it. What I would rather do to solve this issue is do an ng-if or ng-show on the HTML element to only show the a...

I don't like that we're setting the alert status class outside the function that does it. What I would rather do to solve this issue is do an ng-if or ng-show on the HTML element to only show the alert section if there are inventory items.

Is this correct? There is no space between .cce-inventory-item-status and .is-awaiting-repair?

Is this correct? There is no space between .cce-inventory-item-status and .is-awaiting-repair?

OLMIS-4475: add new facility cce status and use in component when no cce, hide alerts
OLMIS-4475: add new facility cce status and use in component when no cce, hide alerts
I'm not sure anymore...

I'm not sure anymore...

I suppose we could, although if we make the inventory items section no longer a popover, it won't make sense anymore.

I suppose we could, although if we make the inventory items section no longer a popover, it won't make sense anymore.

I see that FUNCTIONAL_STATUS constant has it anyway, so I'll use that one. I don't see it referenced anywhere as getFunctionalClassStatus. Where do you see that?

I see that FUNCTIONAL_STATUS constant has it anyway, so I'll use that one. I don't see it referenced anywhere as getFunctionalClassStatus. Where do you see that?

Shouldn't we change the second part to a inventoryItemsPopover then?

Shouldn't we change the second part to a inventoryItemsPopover then?

Chongsun Ahn The getFunctionalClassStatus method. We can do it like this vmm.getFunctionalClassStatus = cceInventoryItemStatusFactory.getFunctionalClassStatus. Why is it called getFunctionalClass...

Chongsun Ahn The getFunctionalClassStatus method. We can do it like this

vmm.getFunctionalClassStatus = cceInventoryItemStatusFactory.getFunctionalClassStatus.

Why is it called getFunctionalClassStatus and not getFunctionalStatusClass by the way?

Which method?

Which method?

I prefer to leave it, as inventoryItems is a category for the popover, and this is a title. If there are additional labels, they would also be under this inventoryItems category.

I prefer to leave it, as inventoryItems is a category for the popover, and this is a title. If there are additional labels, they would also be under this inventoryItems category.

Could we make this function non-anonymous called hasActiveAlerts? I think it should be more readable.

Could we make this function non-anonymous called hasActiveAlerts? I think it should be more readable.

We can drop the title part.

We can drop the title part.

We could simply expose the method itself here.

We could simply expose the method itself here.

I think that we already have method to get this class in functional status constant file, not sure if it should be there or here but we should definitely merge those two. I see that you are using 5...

I think that we already have method to get this class in functional status constant file, not sure if it should be there or here but we should definitely merge those two.
I see that you are using 5 statuses but in the constant file I see only 3, can you fix that?

remove $q

remove $q