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: map refactoring #1837

Merged
merged 8 commits into from
Dec 18, 2023
Merged

feat: map refactoring #1837

merged 8 commits into from
Dec 18, 2023

Conversation

w1nklr
Copy link
Collaborator

@w1nklr w1nklr commented Dec 15, 2023

Refactor Map component to remove numerous useEffect

Main motivation is to fix wrong view state computed when first rendering is done with hidden flag.
First reproduced by application with tabbed layout where all the tabs are rendered using hidden flag.
This use case has been added to the storybook to demonstrate the issue, along with a 'triggerHome' control (in a previous PR).

The current PR brings:

  • onResize() function to handle resizing
  • use a React reducer to handle Z scaling
    Add support for PageUp/PageDown and Shift modifier
  • use a React reducer to compute the global 3D bounding box from the reportBoundingBox callback of the layers
  • introduce a ViewController class to generate the DeckGL views and viewState
    This brings all the computations in one place/one step by providing state change allowing to fine-tune the computations
  • use bounding box from the 'cameraPosition' (if set as zoom field) instead of the data bounding box

Fixes:

  • initial viewState is correct, even when rendered with 'hidden' flag set to true
  • triggerHome now respects current zScale

In the end, the final implementation drops down to 6 React useEffect, from the 16 initial ones !

Fix for #1820

Main motivation is to fix wrong view state computed when first rendering is done with hidden flag.
First reproduced by application with tabbed layout where all the tabs are rendered using hidden flag.
This use case has been added to the storybook to demonstrate the issue, along with a 'triggerHome' control (in a previous PR).

The current PR brings:
- onResize() function to handle resizing
- use a React reducer to handle Z scaling
  Add support for PageUp/PageDown and Shift modifier
- use a React reducer to compute the global 3D bounding box from the reportBoundingBox callback of the layers
- introduce a ViewController class to generate the DeckGL views and viewState
  This brings all the computations in one place/one step by providing state change allowing to fine-tune the computations
- use bounding box from the 'cameraPosition' (if set as zoom field) instead of the data bounding box

Fixes:
- initial viewState is correct, even when rendered with 'hidden' flag set to true
- triggerHome now respects current zScale

In the end, the final implementation drops down to 6 React useEffect,  from the 16 initial ones !
@w1nklr w1nklr requested review from hkfb and nilscb December 15, 2023 15:55
@w1nklr w1nklr merged commit 849b839 into equinor:master Dec 18, 2023
8 checks passed
hkfb pushed a commit that referenced this pull request Dec 18, 2023
@hkfb
Copy link
Collaborator

hkfb commented Dec 18, 2023

🎉 This issue has been resolved in version [email protected] 🎉

The release is available on GitHub release

@hkfb hkfb added the released label Dec 18, 2023
w1nklr added a commit that referenced this pull request Dec 19, 2023
Fix regression in viewport size introduced in PR #1837
hkfb pushed a commit that referenced this pull request Dec 19, 2023
@w1nklr w1nklr deleted the map_refactoring branch December 19, 2023 14:05
w1nklr added a commit that referenced this pull request Jan 9, 2024
Fix missing dependency to recompute camera state
Fix wrong Z range for non mesh grids
Optimize bounding box update when the bounding box is not changing

Some camera computation issues where introduced in PR #1837
hkfb pushed a commit that referenced this pull request Jan 9, 2024
## [0.13.1](https://github.com/equinor/webviz-subsurface-components/compare/[email protected]@0.13.1) (2024-01-09)

### Bug Fixes

* camera computation in some cases ([#1860](#1860)) ([ded2afe](ded2afe)), closes [#1837](#1837)
w1nklr added a commit that referenced this pull request Jan 10, 2024
Fix regression introduced in #1837.

Revert to use initial precedence to compute camera in 2D:
- camera if provided,
- 2D bounds if provided,
- default computation otherwise

Add story to demonstrate how to sync 2 SubSurfaceViewer.
hkfb pushed a commit that referenced this pull request Jan 10, 2024
## [0.4.2](https://github.com/equinor/webviz-subsurface-components/compare/[email protected]@0.4.2) (2024-01-10)

### Bug Fixes

* wrong camera computation in 2D when bounds are provided ([#1863](#1863)) ([3baef9f](3baef9f)), closes [#1837](#1837)
hkfb pushed a commit that referenced this pull request Jan 10, 2024
## [1.1.2](https://github.com/equinor/webviz-subsurface-components/compare/[email protected]@1.1.2) (2024-01-10)

### Bug Fixes

* wrong camera computation in 2D when bounds are provided ([#1863](#1863)) ([3baef9f](3baef9f)), closes [#1837](#1837)
hkfb pushed a commit that referenced this pull request Jan 10, 2024
## [0.13.3](https://github.com/equinor/webviz-subsurface-components/compare/[email protected]@0.13.3) (2024-01-10)

### Bug Fixes

* wrong camera computation in 2D when bounds are provided ([#1863](#1863)) ([3baef9f](3baef9f)), closes [#1837](#1837)
hkfb pushed a commit that referenced this pull request Jan 10, 2024
## [1.0.2](https://github.com/equinor/webviz-subsurface-components/compare/[email protected]@1.0.2) (2024-01-10)

### Bug Fixes

* wrong camera computation in 2D when bounds are provided ([#1863](#1863)) ([3baef9f](3baef9f)), closes [#1837](#1837)
hkfb pushed a commit that referenced this pull request Jan 10, 2024
## [1.3.4](https://github.com/equinor/webviz-subsurface-components/compare/[email protected]@1.3.4) (2024-01-10)

### Bug Fixes

* wrong camera computation in 2D when bounds are provided ([#1863](#1863)) ([3baef9f](3baef9f)), closes [#1837](#1837)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants