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: plugin flashes when interacting with Interpretations modal [DHIS2-15570] #3017

Merged
merged 14 commits into from
Apr 23, 2024

Conversation

jenniferarnesen
Copy link
Collaborator

@jenniferarnesen jenniferarnesen commented Apr 11, 2024

Fixes DHIS2-15570
Fixes DHIS2-17137

Requires dhis2/analytics#1608


Key features

  1. Prevent re-requests for analytics, and resulting re-renders that were giving a flashing effect.

Description

  1. InterpretationModal: memoize the callback to avoid a re-render due to a "new" function being passed with each render of InterpretationModal. The effect of this change is minor, probably just 1 rerender
  2. VisualizationPlugin: ensure that the size of the plugin is set when it is coming from the modal so that when changing the width of the viewport, the visualization adjusts its size accordingly. (Previously this "worked" but only because of all the re-fetch/re-renders)
  3. VisualizationPluginWrapper: need to listen to the individual properties, rather than the whole props object since that was triggering lots of unnecessary re-fetch/renders.
  4. set the max-height to respect viewport so that vertical resize causes the chart to adjust accordingly

Video

before (width):

flashing.in.interp.modal.mov

before (height):

no.height.resize.mov

after (width):
https://github.com/dhis2/data-visualizer-app/assets/6113918/5dbeb7e4-edcd-4f5f-9b4a-e829c9061920

after (height):

height.resize.works.mov

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Apr 11, 2024

@dhis2-bot dhis2-bot temporarily deployed to netlify April 11, 2024 13:55 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify April 11, 2024 14:01 Inactive
Copy link

cypress bot commented Apr 11, 2024

Passing run #2789 ↗︎

0 767 2 0 Flakiness 0

Details:

Merge 4191cf0 into 43f89be...
Project: Data Visualizer App Commit: 7a3ae40251 ℹ️
Status: Passed Duration: 06:20 💡
Started: Apr 22, 2024 10:53 AM Ended: Apr 22, 2024 10:59 AM

Review all test suite changes for PR #3017 ↗︎

@dhis2-bot dhis2-bot temporarily deployed to netlify April 11, 2024 14:41 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify April 17, 2024 12:44 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify April 17, 2024 13:01 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify April 17, 2024 13:07 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify April 17, 2024 13:31 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify April 17, 2024 14:39 Inactive
@jenniferarnesen jenniferarnesen changed the title fix: calculate visualization width and height when it is with the plugin wrapper fix: plugin flashes when interacting with Interpretations modal [DHIS2-15570] Apr 18, 2024
@dhis2-bot dhis2-bot temporarily deployed to netlify April 22, 2024 09:40 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify April 22, 2024 10:44 Inactive
@jenniferarnesen jenniferarnesen merged commit 1799592 into dev Apr 23, 2024
23 checks passed
@jenniferarnesen jenniferarnesen deleted the fix/plugin-flashing-interpretations branch April 23, 2024 07:53
@cypress cypress bot mentioned this pull request Apr 23, 2024
jenniferarnesen added a commit that referenced this pull request Apr 25, 2024
Fixes DHIS2-15570
Fixes DHIS2-17137

Prevent re-requests for analytics, and resulting re-renders that were giving a flashing effect.

* InterpretationModal: memoize the callback to avoid a re-render due to a "new" function
being passed with each render of InterpretationModal.
The effect of this change is minor, probably just 1 rerender

* VisualizationPlugin: ensure that the size of the plugin is set when it is coming
from the modal so that when changing the width of the viewport,
the visualization adjusts its size accordingly. (Previously this "worked" but only
because of all the re-fetch/re-renders)

* VisualizationPluginWrapper: need to listen to the individual properties,
rather than the whole props object since that was triggering lots of
unnecessary re-fetch/renders.

* set the max-height to respect viewport so that vertical resize causes
the chart to adjust accordingly
dhis2-bot added a commit that referenced this pull request Apr 25, 2024
## [100.5.3](v100.5.2...v100.5.3) (2024-04-25)

### Bug Fixes

* plugin flashes when interacting with Interpretations modal ([#3017](#3017)) ([827ef37](827ef37))
* **translations:** sync translations from transifex (dev) ([43f89be](43f89be))
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.

3 participants