Skip to content

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
45 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.

4 participants