Remove check map bbox, add bbox validation #662
Conversation
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
🤔 |
|
If I remember correctly, we needed the check to recenter the viewport when changing layers. If the data changed from one layer (or date) to the other, the user shouldn't be left on an empty map (this would happen on sparse datasets). However most datasets are global and I can't find what prompted the original use case anymore. Perhaps is no longer needed. |
There was a problem hiding this comment.
@hanbyul-here I'm not sure I understand why this check is needed. The bounds don't ever seem to be greater than 180/90. If you switch between datasets with the equirectangular projection (when no user position is set on the url) it will work as expected - removing the checkFitBoundsFromLayer seem to be enough
Do you have some steps I can follow to see what was happening/what this you're trying to fix?
|
@danielfdsilva STAC returns bbox going beyond -180, 180, -90,90 If we don't have a clear use case anymore, I'd like to propose merging this into main. |
|
@hanbyul-here oh, that's weird. so perhaps instead of checking if valid or not, we should clamp it to 180/90. There's no reason (afaik) to have bounds over that. what do you think? |
|
Perhaps there's even something strange in the backend that shouldn't be happening? cc @anayeaye |
|
ok @anayeaye confirmed that there are some datasets that have invalid bbox, and the bbox is coming from the item in the dataset. @ividito added a validation for the extent https://github.com/NASA-IMPACT/veda-stac-ingestor/blob/main/api/src/schema_helpers.py#L34-L45 but it looks like some datasets were directly ingested without being validated. 🤔 |
|
It looks like we don't even need to set the bounds on mdx, (if the data coverage is world-wide, bbox from stac will get clamped, and then the map will fit into the clamped bbox automatically.) 👯 |
## This PR, merged to `ghg` ### Features - Analysis define - New step-by-step layout and various UI tweaks - Added sticky footer, moved submit and cancel buttons there - Added an AoI preset for North America - Datasets are not filtered from `/collections` metadata (spatial, temporal extent) - Dates are now set by default to 2018 - 2022 - Replaced date presets with "2018 - 2022" and "last 10 years" - Date selection not possible anymore before 01-01-1980 - Prevent users from selecting datasets when time range selected would mean fetching too many STAC items - Analysis results page: set a hardcoded limit on the number of items that can be requested as a failsafe mechanism for ☝️ ### Bugfixes - Removed logs in production (US-GHG-Center/veda-config-ghg#152) - Temporary fix of critical error encoutered during demos (#680) - Remove Shadow Scroll as a preventative method for (#691) - Check only day level equity to validate dates (#676) - Remove unneded attributes (#694) - BlockMap initial position error (#687) ### Copy changes - [Updated define copy](f1e7171) - [Disclaimer on results page](54cf01e) --- ## Other PRs, merged to `main` ### Added in veda UI but made configurable (so not affecting veda-config): - #673 - #665 - #664 ### Bugfixes - #672 - #670 / #666 - #662 - #691 - #650 - #676 - #694 - #687
As far as I can read, the code requires the new bound either to totally fit into the current bound or totally outside of the current bound. This blocks me from setting up the initial view as [-180, -90, 180, 90] for this issue: #656 (It looks like Mapbox's default viewport for equirectangular projection is somewhere [-190, -59, 190, 59]... pretty sure it changes depending on the screen size.)
I don't have much background context but I wonder if we really need to check the current bound? (If not, we can just merge this change to
maininstead ofghg- and I will remove the function.) This PR skips this check and just validate the bbox against Mapbox's requirement.