Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mainUI] Proposed to add expanding group items in graph #2757

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jsjames
Copy link
Contributor

@jsjames jsjames commented Sep 15, 2024

The feature to plot all items in a group on a graph is not implemented and missing from the sitemap graph feature. I have prototyped a solution in this PR - but before I spend more time to perfect/test, i wanted to get feedback on the implementation to ensure the overall plan is okay and get any suggestions:

  • I have added a boolean config parameter call "groupItems" which can be set to true to expand the group. If this field does not exist or is set to false, things operate as normal - with similar performance.
  • When this field is set to true, it will wait for the first itemPromises to finish and then expand the neededItems to query the persistence (this wait only happens if the groupItems is set to true - so performance of original graphs should not be affected)
  • Finally, I've modified the oh-time-series get to return multiple series items, one for each of the items in the group. This works well in the line graphs I have tested.

Note, I have not tested other graph types to make sure there is no impact as I want to first get feedback/input on the proposed implementation.

@florian-h05 - any feedback on this?

@jsjames jsjames changed the title Proposed to add expanding group items in graph [main UI] Proposed to add expanding group items in graph Sep 15, 2024
@jsjames jsjames marked this pull request as draft September 15, 2024 20:57
@jsjames jsjames changed the title [main UI] Proposed to add expanding group items in graph [mainUI] Proposed to add expanding group items in graph Sep 15, 2024
@jimtng
Copy link
Contributor

jimtng commented Sep 16, 2024

got a screenshot?

@jsjames
Copy link
Contributor Author

jsjames commented Sep 16, 2024

got a screenshot?

Here's a screenshot - this one still has the item gEnergy on the chart which is the group all of the other items belongs to. The latest code removes the group item.

image

@florian-h05
Copy link
Contributor

Hi @jsjames,

nice feature proposal, you definitely have my support for this!
Wrt to the implementation outlined in the PR description, I think the overall plan is okay 👍
Just one minor thing: I would rather name that new parameter expandGroup.

Let me know when you need help or this PR is ready for review!

@florian-h05 florian-h05 added enhancement New feature or request main ui Main UI labels Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants