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

User defined dataset bounds #596

Merged
merged 7 commits into from
Aug 3, 2023
Merged

Conversation

danielfdsilva
Copy link
Collaborator

This PR adds support for dataset layers to define initial bounds for the map. This is useful for datasets that are not global, and for which the STAC bounds are not appropriate.

Bounds are defined using a bounds property on the dataset layer. This property should be an array with 4 numbers, representing the minimum and maximum longitude and latitude values, in that order.
Example (world bounds)

bounds: [-180, -90, 180, 90] 

Note on bounds and dataset layer switching:
The exploration map will always prioritize the position set in the url. This is so that the user can share a link to a specific location. However, upon load the map will check if the position set in the url is within or overlapping the bounds of the dataset layer. If it is not, the map will switch to the dataset layer bounds. This is to avoid showing an empty map when the user shares a link to a location that is not within the dataset layer bounds.
If there are no bounds set in the dataset configuration, the bbox from the STAC catalog will be used if available, otherwise it will default to the world bounds.

@netlify
Copy link

netlify bot commented Jul 27, 2023

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit f654526
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/64ca8600fffb810008984819
😎 Deploy Preview https://deploy-preview-596--veda-ui.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@nerik nerik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍
As a note for later, we'll have to figure out what happens when we have multiple layers and more than one wants a bbox 😬

docs/content/frontmatter/layer.md Outdated Show resolved Hide resolved
];

if (checkFitBoundsFromLayer(usableBounds, mapInstance)) {
mapInstance.fitBounds(usableBounds, { padding: FIT_BOUNDS_PADDING });
Copy link
Contributor

@nerik nerik Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is the same as what you added in vector-timeseries.
Might be worth exploring running it at the map level (maybe in mapbox/index.tsx), or as an alternative provide a hook to reduce repetition.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Abstracted the function to a hook. There should be less repetition now, but I had to get the stac bbox for the vector data as well.

@j08lue
Copy link
Contributor

j08lue commented Jul 27, 2023

This is useful for datasets that are not global, and for which the STAC bounds are not appropriate.

For the record, the most prominent use case for these bounds is when the dataset does cover a larger area (e.g. global), but the Dashboard instance has a thematic / contextual focus on a specific region.

For example, the U.S. GHG Center has a focus on the US, but instead of subsetting or limiting the larger/global datasets to that region, they just want to provide an initial / default view centered on the focus region (US / CONUS).

Copy link
Contributor

@j08lue j08lue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, one concern with the current behavior: Say a user opens the NO2 PT dataset, which is the global NO2 dataset but with bounds set on Portugal. They then zoom to another region and want to share that view with another user by giving them the URL. The other users will not be able to see the same view - the link to the NO2 PT view will always zoom to Portugal after loading.

For the GHG Center, we will basically set the "initial bounds" to CONUS for all datasets on record (since it is the U.S. GHG Center), but the behavior that you cannot share links that point outside of that area (even if data exists) will probably surprise some users.

As I understand it, you implemented bounds to really override the bounds coming from STAC in all aspects - also as the source of truth of what the dataset actually covers / should cover.

I think the configured initial/default bounds (initial_bounds?) should still allow for users to zoom outside of that area and create URLs that point to it. I know we are doing this to prevent users from going to areas where there is no data - but perhaps we can do without these guardrails for the case where a particular area is requested via the URL parameters?

docs/content/frontmatter/layer.md Outdated Show resolved Hide resolved
@danielfdsilva
Copy link
Collaborator Author

danielfdsilva commented Jul 28, 2023

@j08lue That's a very valid point and one that merits some thought.
There are some conflicting sources competing here, and that makes it challenging to pick which one to use.

Whenever the map moves the url gets updated with the position, which means that there will almost always be a value in the url. If, right now, we rely on the existence of that value to decide whether to fit the bounds or not, we'll never do it because there's always a value.


Possible approach

It could be possible to only set the value on the url if the user moves the map, which means that if the value is there it was by user's intention and so that value should take precedence.

Now we have to think about what happens when the user changes between dataset layers. Two options:

Option 1:

  • Whenever the user changes layer the viewport gets recentered on that layer's bounds.
    • Cons: It will not be possible to switch between layers while maintaining the existent viewport (if outside the bounds)

Option 2:

  • Once the user moves the map, the viewport is set and changing between layers does nothing
    • Cons: The only way to recenter a dataset on its bounds would be to reset the url manually

If all the layers of a dataset have the same bounds, then option 2 gives the user more freedom, given that it is expected that the browsing that they'd be doing, is all in the same area - but I don't think we can assume this.


Future (A&E)

This is not going to be viable in the new A&E. Since arbitrary datasets can be added we have no control (nor should force)
which bounds get set. What we could do is provide a way for the user to manually "zoom to dataset bounds".

@j08lue
Copy link
Contributor

j08lue commented Jul 28, 2023

Right, this bounds logic is one of the few automagic pieces of the Exploration view, I guess, so it needs careful design to not produce unexpected results for the users.

Maybe we should for now just stick to the implementation you made here, I think: bounds in the config overrule the bounds from STAC, all app behavior stays the same. Then we can rework the logic later, if we think we should.

Alternatively, we could stop using bounds for anything except for initialization (of the URL viewport, which then always is respected). Once we start having multiple visible / active datasets at initialization, we may need to calculate the maximum viewport for all of them, to have a sensible initial viewport.

That would basically be how QGIS does it: when you load your first layers on a blank canvas, it zooms to the combined extent of those. From then on, the viewport stays fixed, even if you add new layers that do not intersect the current viewport.

I think switching between different layers while looking at the same spot on the map is a very common workflow for getting a multi-parameter view of something, or for comparing geometries - but this may be biased by my workflows.

In QGIS, I also often use "zoom to layer(s)" to change viewport when working with multiple layers that cover disjoint areas:

image

But I am sure there were considerations that lead to the current behavior, that I am not aware of. So maybe not change the behavior now and thoroughly review that later?

Copy link
Collaborator

@hanbyul-here hanbyul-here left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question For my education, so we can not use the bounding box just to initialize the page? (when mapPosition value is empty?)

@j08lue
Copy link
Contributor

j08lue commented Jul 28, 2023

so we can not use the bounding box just to initialize the page

We could, I guess. That would mean removing the other logic that is currently in place and simplifying the behavior - effectively removing the features that guide (force) the users towards the area defined by bounds. I'd be fine with that, but have respect for any reasons for having that logic that I am not aware of.

@hanbyul-here
Copy link
Collaborator

My apologies that my comment was not helpful. It sounds like the bound is not even a bound for the dataset; it is just for the display on the landing page. then, this aligns more with initialMapPosition value rather than the boundary of the dataset, I think. (something like what a user passes to Chapter in scrollymap block). I should rephrase my question: Can we treat this boundary as the initial map position/ and not mix it with the boundary data from STAC? (I think this is what you are suggesting for the future @j08lue? I mainly wanted to know if there are also tradeoffs from this approach for my education.)

@danielfdsilva
Copy link
Collaborator Author

@hanbyul-here I agree that these bounds are more like an initial map position for this dataset. You could also view these use defined bounds and the dataset bounds in the context of the exploration that was defined by the user, i.e. for this specific configuration only those bounds are of interested. But I digress.

Currently the dataset default bounds, only become available after the dataset has loaded info from STAC, at which point the mapPosition is no longer empty, making in tricky to know which to rely on.

@danielfdsilva
Copy link
Collaborator Author

@hanbyul-here @j08lue added an experimental commit with the "option 2" described above.
The map will fit bounds only until there is not a position in the url, which means that you'll be able to toggle between No2 PT and US, until you move the map.

I think this kind of solves for that situation where the map is centered somewhere when the users open the page, but the url can still be shared and datasets switched quickly without loosing context.

Check it out in the preview url and let me know what your think

@j08lue
Copy link
Contributor

j08lue commented Jul 31, 2023

Check it out in the preview url and let me know what your think

I think this is a great solution that I would be happy for us to release. We can collect user feedback on the live application.

@danielfdsilva danielfdsilva merged commit 1ced268 into main Aug 3, 2023
8 checks passed
@danielfdsilva danielfdsilva deleted the feature/predefined-bounds branch August 3, 2023 09:09
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