Refactor/better performance

[Corstor]

No description provided.

[Ventus218]

I'm currently reviewing this.

Regarding DeviceGroupsDialog, I think that its logic should be decoupled from the groups store.
(I think it would be better to have a groups store that is then used by a "deviceGroupsDialog" store which specifically implements the logic about the dialog.)
I think that method names in stores/groups.ts are not really clear:

• getDeviceFromGroups can just be called findDevice (the fact that you are searching the device inside groups is implicit in the context)
• deviceGroups could be split into two methods:
  1. a computed property "selectedDeviceGroups" which can be used by the dialog to get all the groups it should present
  2. a method "showGroupsOfDevice(DeviceId)" which sets selectedDevice (consecutively also updates the computed property above)

Moreover I would not call the loading overlay from inside the stores (example here), I think it's not flexible enough and it also couples logic with presentation, it is highly probable that views will have their own way of dealing with loading screens.

Tell me what you think about this.

[Corstor]

I'm currently reviewing this.

Regarding DeviceGroupsDialog, I think that its logic should be decoupled from the groups store. (I think it would be better to have a groups store that is then used by a "deviceGroupsDialog" store which specifically implements the logic about the dialog.) I think that method names in stores/groups.ts are not really clear:

* getDeviceFromGroups can just be called findDevice (the fact that you are searching the device inside groups is implicit in the context)

* deviceGroups could be split into two methods:
  
  1. a computed property "selectedDeviceGroups" which can be used by the dialog to get all the groups it should present
  2. a method "showGroupsOfDevice(DeviceId)" which sets selectedDevice (consecutively also updates the computed property above)

Moreover I would not call the loading overlay from inside the stores (example here), I think it's not flexible enough and it also couples logic with presentation, it is highly probable that views will have their own way of dealing with loading screens.

Tell me what you think about this.

The thing is, I also need to access groups of a device in parts that won't use the dialog, so I do not really understant that division of deviceGroups, because "showGroupsOfDevice" seems to me that will show the dialog? I will try instead to do something in between, having the logic in the group store and just show/not show the dialog in another store, is that ok?

To be more specific, selectedDevice in my code is used just by the dialog, selected groups is used also from other parts.
Moreover I do not understand why the need of having a computed property that needs the setter of selectedDevice, precisely because I need selected groups by a deviceId and not to also set a selectedDevice

[Corstor]

I'm currently reviewing this.
Regarding DeviceGroupsDialog, I think that its logic should be decoupled from the groups store. (I think it would be better to have a groups store that is then used by a "deviceGroupsDialog" store which specifically implements the logic about the dialog.) I think that method names in stores/groups.ts are not really clear:

* getDeviceFromGroups can just be called findDevice (the fact that you are searching the device inside groups is implicit in the context)

* deviceGroups could be split into two methods:
  
  1. a computed property "selectedDeviceGroups" which can be used by the dialog to get all the groups it should present
  2. a method "showGroupsOfDevice(DeviceId)" which sets selectedDevice (consecutively also updates the computed property above)

Moreover I would not call the loading overlay from inside the stores (example here), I think it's not flexible enough and it also couples logic with presentation, it is highly probable that views will have their own way of dealing with loading screens.
Tell me what you think about this.

The thing is, I also need to access groups of a device in parts that won't use the dialog, so I do not really understant that division of deviceGroups, because "showGroupsOfDevice" seems to me that will show the dialog? I will try instead to do something in between, having the logic in the group store and just show/not show the dialog in another store, is that ok?

To be more specific, selectedDevice in my code is used just by the dialog, selected groups is used also from other parts. Moreover I do not understand why the need of having a computed property that needs the setter of selectedDevice, precisely because I need selected groups by a deviceId and not to also set a selectedDevice

Solution proposed: now the store about groups (logic) just works with all the groups and getting groups of a device, and another store about groups dialog (presentation) works with a computed property selectedDeviceGroups which is like the one proposed, and a "setter" showDeviceGroups that works as the one proposed (set a deviceId used then by the computed property). In this way the dialog subscribes itself to the groups-dialog store selectedDevice ref.
The groups dialog store uses the groups store to get the selected device groups.

[Ventus218]

We can make these 4 calls much faster if we execute them in parallel instead of sequentially (using Promise.all if I recall correctly)

[Corstor]

Done!

[Ventus218]

Can you check that loadingOverlay.start/stop is added back again in all pages where you removed it, I think some might be missing it.

I've already reviewed most of the files, once you fixed this I'll finish the review!

[Ventus218]

Solution proposed: now the store about groups (logic) just works with all the groups and getting groups of a device, and another store about groups dialog (presentation) works with a computed property selectedDeviceGroups which is like the one proposed, and a "setter" showDeviceGroups that works as the one proposed (set a deviceId used then by the computed property). In this way the dialog subscribes itself to the groups-dialog store selectedDevice ref.

The groups dialog store uses the groups store to get the selected device groups.

It's good now!
This is what i had in mind, i may not have been 100% clear in my description but you got it!

[Corstor]

I didn't understand this comment... Do you talk about some pages in which i took out the loadingOverlay that are not the stores? If so, I did it because now those pages do not call the server anymore but totally rely on the stores

Again here, maybe I'm not getting the context 100%, so I'll trust you

[github-actions]

🎉 This PR is included in version 0.4.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀