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

feat: implement shared dashboard plugin wrapper #1672

Closed
wants to merge 32 commits into from

Conversation

edoardo
Copy link
Member

@edoardo edoardo commented Jun 4, 2024

Relates to dhis2/data-visualizer-app#3082
Relates to dhis2/line-listing-app#396
Relates to dhis2/maps-app#3232


Key features

  1. implement a shared dashboard plugin wrapper component

Description

This wrapper adds support for dashboard offline caching.
Before the logic was replicated in the plugin component of each analytics app.
Now all apps can use this wrapper and only implement their specific logic related to the props required to be passed down to the plugin rendering component.


Known issues

The Storybook build command fails has been updated in a separate PR and works.

It used to run automatically after the build command, so also in Github, which I'm not sure is necessary.
It has been removed in the PR just to be able to build and copy the artifact on d2-ci so it can be used in app's PRs.

I think the issue is related to the upgrade of cli-app-scripts which introduces Webpack 5, while the Storybook setup is for Webpack 4.

I'm not sure an upgrade of Storybook belongs to this PR, but after this PR is merged, Storybook is broken.

@HendrikThePendric
Copy link
Contributor

Since I don't think we actually use the storybook build anywhere, I think it is fine to not include a fix for it in this PR. We should probably create a new JIRA ticket with a slightly larger scope: both building the storybook, and then also publishing it somewhere (perhaps a netlify deploy?)

Copy link
Contributor

@HendrikThePendric HendrikThePendric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left some comments and am not sure if we should perhaps not merge this PR until that bug in the useCacheableSection hook is solved. But will approve for logistical reasons.

@edoardo edoardo force-pushed the feat/dashboard-plugin-wrapper branch from 7c8d2c8 to 87873cf Compare July 12, 2024 11:51
@edoardo edoardo force-pushed the feat/dashboard-plugin-wrapper branch from 87873cf to 1615bb4 Compare August 5, 2024 09:04
@edoardo edoardo force-pushed the feat/dashboard-plugin-wrapper branch from 1a5c9ef to 292f88f Compare August 9, 2024 09:10
@edoardo
Copy link
Member Author

edoardo commented Aug 16, 2024

Since I don't think we actually use the storybook build anywhere, I think it is fine to not include a fix for it in this PR. We should probably create a new JIRA ticket with a slightly larger scope: both building the storybook, and then also publishing it somewhere (perhaps a netlify deploy?)

I created a task for this so we (hopefully) don't forget.

This has been done and merged already, see PR #1724 .

@jenniferarnesen
Copy link
Collaborator

jenniferarnesen commented Jan 27, 2025

Superceded by #1748 because of unfortunate dependency downgrades that were introduced due to regenerating the yarn.lock file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants