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

fix: inject style in props passed to VisualizationPlugin (DHIS2-15126) #2322

Merged
merged 9 commits into from
Jun 26, 2023

Conversation

edoardo
Copy link
Member

@edoardo edoardo commented May 25, 2023

Fixes DHIS2-15126

Key features

  1. pass style to VisualizationPlugin

Description

style is used to compute the correct width when toggling the legend panel in dashboard.
style is not passed in props from dashboard because doing so can cause re-rendering of the whole iframe tag which causes refetching of the plugin and everything else (analytics included) (see: dhis2/dashboard-app#2247).
Instead, style can be computed directly in the PluginWrapper based on the parent element dimensions, these should match with width and height set on the iframe tag.


Known issues

  1. browser window or dashboard item resize causes the legend to remain in the same position, resulting in either too much space on the right side or cutoff Fixed with the resize observer

Screenshots

Before:
Screenshot 2023-05-25 at 15 33 34

After:
Screenshot 2023-05-25 at 15 33 52

@dhis2-bot
Copy link
Contributor

dhis2-bot commented May 25, 2023

@dhis2-bot dhis2-bot temporarily deployed to netlify May 25, 2023 13:39 Inactive
@cypress
Copy link

cypress bot commented May 25, 2023

Passing run #2158 ↗︎

0 637 0 0 Flakiness 0

Details:

Merge 5e045a3 into 0655e59...
Project: Data Visualizer App Commit: 33af055de2 ℹ️
Status: Passed Duration: 10:36 💡
Started: Jun 26, 2023 9:46 AM Ended: Jun 26, 2023 9:56 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@edoardo edoardo marked this pull request as draft May 25, 2023 14:21
@dhis2-bot dhis2-bot temporarily deployed to netlify May 26, 2023 14:18 Inactive
@edoardo edoardo marked this pull request as ready for review May 30, 2023 12:47
@dhis2-bot dhis2-bot temporarily deployed to netlify May 30, 2023 12:50 Inactive
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 left one remark about variable naming, which is no biggie. Other than that, this looks like a good solution to me 👍 .

src/components/VisualizationPlugin/VisualizationPlugin.js Outdated Show resolved Hide resolved
edoardo added 4 commits May 31, 2023 11:35
style is used to compute the correct width when toggling the legend
panel in dashboard.
style is not passed in props from dashboard because doing so can cause
re-rendering of the whole iframe tag which causes refetching of the
plugin and everything else (analytics included).
This is because eventually the PluginWrapper component will be replaced
by the generic one from app-platform.
If possible all custom logic should not be put in other components at
the app level.
It's a callback ref, not a normal ref.
@edoardo edoardo force-pushed the fix/legend-positioning-in-dashboard branch from 5ccad9d to 388b5b9 Compare May 31, 2023 09:35
@dhis2-bot dhis2-bot temporarily deployed to netlify May 31, 2023 09:43 Inactive
@janhenrikoverland janhenrikoverland changed the title fix: inject style in props passed to VisualizationPlugin fix: inject style in props passed to VisualizationPlugin (DHIS2-15126) Jun 9, 2023
@dhis2-bot dhis2-bot temporarily deployed to netlify June 9, 2023 08:56 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify June 9, 2023 09:46 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify June 12, 2023 11:51 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify June 19, 2023 07:38 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify June 26, 2023 07:19 Inactive
@edoardo edoardo merged commit 6908735 into dev Jun 26, 2023
@edoardo edoardo deleted the fix/legend-positioning-in-dashboard branch June 26, 2023 09:59
janhenrikoverland added a commit that referenced this pull request Aug 10, 2023
#2322)

* fix: inject style in props passed to VisualizationPlugin

style is used to compute the correct width when toggling the legend
panel in dashboard.
style is not passed in props from dashboard because doing so can cause
re-rendering of the whole iframe tag which causes refetching of the
plugin and everything else (analytics included).

* refactor: move the size logic from the plugin wrapper

This is because eventually the PluginWrapper component will be replaced
by the generic one from app-platform.
If possible all custom logic should not be put in other components at
the app level.

* fix: observe size changes for legend positioning

* refactor: rename const for clarity

It's a callback ref, not a normal ref.

---------

Co-authored-by: Jan Henrik Øverland <[email protected]>
Co-authored-by: Martin <[email protected]>
janhenrikoverland added a commit that referenced this pull request Aug 15, 2023
#2322)

* fix: inject style in props passed to VisualizationPlugin

style is used to compute the correct width when toggling the legend
panel in dashboard.
style is not passed in props from dashboard because doing so can cause
re-rendering of the whole iframe tag which causes refetching of the
plugin and everything else (analytics included).

* refactor: move the size logic from the plugin wrapper

This is because eventually the PluginWrapper component will be replaced
by the generic one from app-platform.
If possible all custom logic should not be put in other components at
the app level.

* fix: observe size changes for legend positioning

* refactor: rename const for clarity

It's a callback ref, not a normal ref.

---------

Co-authored-by: Jan Henrik Øverland <[email protected]>
Co-authored-by: Martin <[email protected]>
janhenrikoverland added a commit that referenced this pull request Aug 15, 2023
#2322)

* fix: inject style in props passed to VisualizationPlugin

style is used to compute the correct width when toggling the legend
panel in dashboard.
style is not passed in props from dashboard because doing so can cause
re-rendering of the whole iframe tag which causes refetching of the
plugin and everything else (analytics included).

* refactor: move the size logic from the plugin wrapper

This is because eventually the PluginWrapper component will be replaced
by the generic one from app-platform.
If possible all custom logic should not be put in other components at
the app level.

* fix: observe size changes for legend positioning

* refactor: rename const for clarity

It's a callback ref, not a normal ref.

---------

Co-authored-by: Jan Henrik Øverland <[email protected]>
Co-authored-by: Martin <[email protected]>
janhenrikoverland added a commit that referenced this pull request Aug 15, 2023
* chore: run workflow release job on tags as well (#2386)

Tag builds were not triggering creation of corresponding branch in d2-ci repo.
The core apps-to-bundle depends on these.

* test: add Cypress tests for SV icon (DHIS2-10496) (#2372)

* fix(translations): sync translations from transifex (dev)

Automatically merged.

* fix(translations): sync translations from transifex (dev)

Automatically merged.

* fix(translations): sync translations from transifex (dev)

Automatically merged.

* test: more tests for custom calculations (DHIS2-13871) (#2287)

* test: rename AO-title to titlebar (DHIS2-15063) (#2435)

* fix(translations): sync translations from transifex (dev)

Automatically merged.

* chore: run nightly at 6:20 instead of 2:20 to avoid concurrently running with instances reset (#2500)

* test: extend timeout after delete has been triggered (#2499)

* fix: inject style in props passed to VisualizationPlugin (DHIS2-15126) (#2322)

* fix: inject style in props passed to VisualizationPlugin

style is used to compute the correct width when toggling the legend
panel in dashboard.
style is not passed in props from dashboard because doing so can cause
re-rendering of the whole iframe tag which causes refetching of the
plugin and everything else (analytics included).

* refactor: move the size logic from the plugin wrapper

This is because eventually the PluginWrapper component will be replaced
by the generic one from app-platform.
If possible all custom logic should not be put in other components at
the app level.

* fix: observe size changes for legend positioning

* refactor: rename const for clarity

It's a callback ref, not a normal ref.

---------

Co-authored-by: Jan Henrik Øverland <[email protected]>
Co-authored-by: Martin <[email protected]>

* chore: increase cypress default timeout for DOM activity (#2549)

* chore: manually bump deps (#2543)

* chore: advance the schedule for the nightly run (#2575)

* test: prevent test names from being dynamic (#2576)

* chore: disable the scheduled nightly test run (#2602)

The servers are currently slow and there will be very little frontend or backend activity during July 2023

* chore: upgrade cypress to v12 and adjust project to it

* chore: remove videos

* chore: gitignore cypress videos

* chore: switch test server to debug (test.e2e is broken/slow)

* chore: revert the test server changes and move to a separate PR

* test: comment out icon tests for currently unsupported types

* test: skip legend test while data element icons are unsupported

* fix(translations): sync translations from transifex (dev)

Automatically merged.

* feat: use Toolbar and ToolbarSidebar from analytics (#2358)

* feat: use Toolbar and ToolbarSidebar from analytics

* feat: use UpdateButton and HoverMenubar from analytics

* feat: update download menu to use hovermenu components from analytics

* feat: use hover menu components from analytics for options menu

* feat: use interpretations button from analytics

* chore: update pot file

* chore: upgrade @dhis2/analytics to latest

* chore: fix typo in component name

* fix: adjust e2e element slectors

* chore: fix missing and redundant imports

* chore: fix odd linter error which only fires on ci

* fix: adjust data test name

* chore: fix lines e2e test

* fix: adjust closeFileMenuWithClick command

* chore: fix failing legens e2e test

* chore: fix legend e2e test even better

* chore: fix scatter e2e

* chore: fix icon e2e

* chore: fix start e2e

* chore: clean up unused imports

* fix: remove divider from plain data source submenu

* chore: upgrade @dhis2/analytics to get toolbar UI improvements

* fix: tweak viz type selector styles to match toolbar

* chore: remove yarn start command which was only used for development

* chore: upgrade analytics to get decreased padding

* fix: ensure `MenuSectionHeader` is `dense` when in `HoverMenuBar`

---------

Co-authored-by: Jan Henrik Øverland <[email protected]>

* Merge pull request #2613 from dhis2/chore/upgrade-cypress-to-v12

chore: upgrade cypress to v12

* chore: run nightly at 6:20 to avoid concurrently running with instances reset (#2500)

* test: extend timeout after delete has been triggered (#2499)

* fix: inject style in props passed to VisualizationPlugin (DHIS2-15126) (#2322)

* fix: inject style in props passed to VisualizationPlugin

style is used to compute the correct width when toggling the legend
panel in dashboard.
style is not passed in props from dashboard because doing so can cause
re-rendering of the whole iframe tag which causes refetching of the
plugin and everything else (analytics included).

* refactor: move the size logic from the plugin wrapper

This is because eventually the PluginWrapper component will be replaced
by the generic one from app-platform.
If possible all custom logic should not be put in other components at
the app level.

* fix: observe size changes for legend positioning

* refactor: rename const for clarity

It's a callback ref, not a normal ref.

---------

Co-authored-by: Jan Henrik Øverland <[email protected]>
Co-authored-by: Martin <[email protected]>

* chore: increase cypress default timeout for DOM activity (#2549)

* chore: manually bump deps (#2543)

* chore: advance the schedule for the nightly run (#2575)

* test: prevent test names from being dynamic (#2576)

* chore: disable the scheduled nightly test run (#2602)

Servers are currently slow. Will be little activity during July 2023.

* chore: upgrade cypress to v12 and adjust project to it

* chore: remove videos

* chore: gitignore cypress videos

* chore: switch test server to debug (test.e2e is broken/slow)

* chore: revert the test server changes and move to a separate PR

* test: comment out icon tests for currently unsupported types

* test: skip legend test while data element icons are unsupported

* fix(translations): sync translations from transifex (dev)

Automatically merged.

* feat: use Toolbar and ToolbarSidebar from analytics (#2358)

* feat: use Toolbar and ToolbarSidebar from analytics

* feat: use UpdateButton and HoverMenubar from analytics

* feat: update download menu to use hovermenu components from analytics

* feat: use hover menu components from analytics for options menu

* feat: use interpretations button from analytics

* chore: update pot file

* chore: upgrade @dhis2/analytics to latest

* chore: fix typo in component name

* fix: adjust e2e element slectors

* chore: fix missing and redundant imports

* chore: fix odd linter error which only fires on ci

* fix: adjust data test name

* chore: fix lines e2e test

* fix: adjust closeFileMenuWithClick command

* chore: fix failing legens e2e test

* chore: fix legend e2e test even better

* chore: fix scatter e2e

* chore: fix icon e2e

* chore: fix start e2e

* chore: clean up unused imports

* fix: remove divider from plain data source submenu

* chore: upgrade @dhis2/analytics to get toolbar UI improvements

* fix: tweak viz type selector styles to match toolbar

* chore: remove yarn start command which was only used for development

* chore: upgrade analytics to get decreased padding

* fix: ensure `MenuSectionHeader` is `dense` when in `HoverMenuBar`

---------

Co-authored-by: Jan Henrik Øverland <[email protected]>

* Merge pull request #2613 from dhis2/chore/upgrade-cypress-to-v12

chore: upgrade cypress to v12

---------

Co-authored-by: Jen Jones Arnesen <[email protected]>
Co-authored-by: Martin <[email protected]>
Co-authored-by: @dhis2-bot <[email protected]>
Co-authored-by: Edoardo Sabadelli <[email protected]>
Co-authored-by: HendrikThePendric <[email protected]>
Co-authored-by: Hendrik de Graaf <[email protected]>
dhis2-bot added a commit that referenced this pull request Aug 15, 2023
# [100.2.0](v100.1.4...v100.2.0) (2023-08-15)

### Features

* 100.2.0 ([#2741](#2741)) ([00b3636](00b3636)), closes [#2386](#2386) [#2372](#2372) [#2287](#2287) [#2435](#2435) [#2500](#2500) [#2499](#2499) [#2322](#2322)
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.

5 participants