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.

Sign up for GitHub

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

Jump to bottom

chore: replace built-in dimension panel and dialog components with d2-ui-analytics shared components #260

Merged
merged 19 commits into from
Apr 26, 2019
Merged

chore: replace built-in dimension panel and dialog components with d2-ui-analytics shared components #260

merged 19 commits into from
Apr 26, 2019

Conversation

edoardo
Copy link
Member

@edoardo edoardo commented Apr 16, 2019 •
edited
Loading

Remaining tasks:

  • decide what to do about the disabled addMetadata call (see comments below)

@edoardo edoardo added the WIP label Apr 16, 2019
@edoardo
chore(dialog-manager): interface Redux store with shared components
cb0621a 
The metadata handling needs more testing. I had to comment out the
action call in Visualization because it overrides some metadata
previously set and breaks (at least) the OrgUnit dimension dialog (ou path
goes missing but is required by the org unit tree)
@jenniferarnesen
Copy link
Collaborator

Comment from Edoardo:

"I had to remove the call to add metadata when the analytics response comes back as it overrides some metadata info that is previously set and required data goes missing (ie. ou path) causing breakage in OrgUnitDimension (at least). For testing you need latest d2-ui-analytics"

@jenniferarnesen
Copy link
Collaborator

jenniferarnesen commented Apr 16, 2019 •
edited
Loading

Some thoughts:

  • what about keeping the original DV DimensionsPanel file and set up all the redux for the panel there? App.js is pretty big already.
  • so the options context menu will remain in the apps and not be part of the shared component?
  • in DialogManager, why are the peIds and dxIds props needed? They seem to be available in the selectedItems prop.

@edoardo
Copy link
Member Author

edoardo commented Apr 24, 2019

  • in DialogManager, why are the peIds and dxIds props needed? They seem to be available in the selectedItems prop.

Yes, they are and I did consider removing those 2 props, but then I saw they were used also for comparing prevProps with props. We could remove them and use selectedItems instead.

@edoardo
Fix handling of org units metadata/parent graph map
f147fd6
@edoardo
Optimize metadata added to store
3c4e32f 
This is to avoid having unnecessary data that bloats the store and
also have the same data structure for all the items.
Reduced also the number of calls to the addMetadata action by passing
multiple keys instead of one at a time.
@edoardo
Copy link
Member Author

edoardo commented Apr 25, 2019 •
edited
Loading

Just to comment on the metadata issue.
I changed the code a bit to use metadata and parentGraphMap in the Redux store as before.
metadata has been optimised to avoid storing information that we don't need.
For orgunits specifically, the path is handled in parentGraphMap.
For all dimensions we only store id, name and displayName in metadata; the current need is just for the dimension's name/displayName lookup by id.
Whenever a list of dimension items is processed (ie. when analytics responses come back) only 1 addMetadata action is fired setting the data for all the items in once.

@jenniferarnesen jenniferarnesen changed the title WIP chore: Use shared components, define dim panel context menu in app chore: Use shared components, define dim panel context menu in app Apr 25, 2019
@jenniferarnesen jenniferarnesen changed the title chore: Use shared components, define dim panel context menu in app chore: replace built-in dimension panel and dialog components with d2-ui-analytics shared components Apr 25, 2019
@jenniferarnesen jenniferarnesen merged commit b520a65 into master Apr 26, 2019
@jenniferarnesen jenniferarnesen deleted the chore/shared-components branch April 26, 2019 06:12
jenniferarnesen pushed a commit that referenced this pull request Apr 26, 2019
@edoardo @jenniferarnesen
chore: replace built-in dimension panel and dialog components with d2…
b46a172 
…-ui-analytics shared components (#260)

This commit replaces the built-in DimensionsPanel and Dimension dialogs with those from the d2-ui-analytics shared library. The shared components are also used by Dashboards-app.

Some changes regarding saving to metadata store were needed: 
* This is to avoid having unnecessary data that bloats the store and
also have the same data structure for all the items.
* Reduced also the number of calls to the addMetadata action by passing
multiple keys instead of one at a time.
jenniferarnesen added a commit that referenced this pull request Apr 26, 2019
@jenniferarnesen
chore: replace built-in components with d2-ui-analytics shared compon…
3798574 
…ents [v32] (#260) (#275)

* chore: replace built-in dimension panel and dialog components with d2-ui-analytics shared components (#260)

This commit replaces the built-in DimensionsPanel and Dimension dialogs with those from the d2-ui-analytics shared library. The shared components are also used by Dashboards-app.

Some changes regarding saving to metadata store were needed: 
* This is to avoid having unnecessary data that bloats the store and
also have the same data structure for all the items.
* Reduced also the number of calls to the addMetadata action by passing
multiple keys instead of one at a time.

* fix: org unit selector - only act on path if it exists (#276)

Fixes include:

* call "Set" instead of "Add" since the returned list contains the complete selected list
* check that ou.path exists before acting on it. In the case of user org units, path does not exist, which resulted in the app crashing

* upgrade d2-ui-analytics
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 33.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

This was referenced Jun 8, 2020
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Jun 17, 2020
Closed
Closed
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 33.1.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenniferarnesen jenniferarnesen jenniferarnesen approved these changes

Assignees
No one assigned
Labels
released on @33.x released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@edoardo @jenniferarnesen @dhis2-bot