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(widgets): Remove onViewportChange Update Guard for Widgets #9303

Merged
merged 3 commits into from
Dec 19, 2024

Conversation

chrisgervang
Copy link
Collaborator

@chrisgervang chrisgervang commented Dec 18, 2024

For #9278

Background

This PR addresses an issue in onViewportChange where an optimization can prevent widgets from receiving a viewport. The issue was first noticed in the react environment because lastViewports is set before all widgets are fully mounted, leaving some widgets in an uninitialized state.

To resolve this, the guard logic in onViewportChange has been removed and the responsibility for handling performance concerns, such as avoiding excessive calls to onViewportChange, is pushed to individual widgets.

Change List

  • Removed guard conditions in onViewportChange that previously compared lastViewports.
  • Prevent excessive updates to the Compass widget when viewports change.

Alternative Considered

Widget Manager Updates Viewports: Fully delegate viewport updates to the widget manager by comparing state directly on the widget instead of relying on lastViewports.

  • Drawbacks: Assumes all widgets store viewports in a consistent format, introducing unwanted rigidity.
  • lastViewports would still remain, as it is used by the manager for other purposes.

This approach was ultimately not chosen to avoid adding required elements to the Widget interface.

@chrisgervang chrisgervang changed the title remove widget manager viewport optimization feat(widgets): remove viewport optimization from widget manager Dec 18, 2024
@chrisgervang chrisgervang changed the title feat(widgets): remove viewport optimization from widget manager feat(widgets): Remove onViewportChange Update Guard for Widgets Dec 18, 2024
@coveralls
Copy link

coveralls commented Dec 18, 2024

Coverage Status

coverage: 91.701% (-0.006%) from 91.707%
when pulling ea1f7b4 on chr/remove-widget-manager-optimization
into 05bca74 on master.

@chrisgervang chrisgervang mentioned this pull request Dec 19, 2024
36 tasks
@chrisgervang chrisgervang merged commit 2374bc7 into master Dec 19, 2024
4 checks passed
@chrisgervang chrisgervang deleted the chr/remove-widget-manager-optimization branch December 19, 2024 23:18
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