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

GHG -> main #679

Merged
merged 66 commits into from
Oct 12, 2023
Merged

GHG -> main #679

merged 66 commits into from
Oct 12, 2023

Conversation

nerik
Copy link
Contributor

@nerik nerik commented Sep 27, 2023

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

Copy changes


Other PRs, merged to main

Added in veda UI but made configurable (so not affecting veda-config):

Bugfixes

hanbyul-here and others added 30 commits September 22, 2023 13:06
This reverts commit 5c7a87d.
hanbyul-here and others added 16 commits October 4, 2023 14:58
Handles issue in
#682 (comment)


While putting this fix, I noticed that `use-media-query` returns
document body size, which differs from how CSS media query works. This
results in a weird style in between 991 px ~ 1000 px. (screenshot
attached, it is on ghg.center now) 👉 I didn't fix this problem, but I
patched it with CSS.

(Eventually, we would need to move to Container query instead of media
query:
https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Container_Queries)

![Screen Shot 2023-10-04 at 3 01 10
PM](https://github.com/NASA-IMPACT/veda-ui/assets/4583806/215d0515-e4ff-4b42-b0b1-63a21506ad1c)
The date gets set back when I try to change the date for Plume datasets:
https://ghg-demo.netlify.app/data-catalog/emit-ch4plume-v1/explore?projection=mercator%7C%7C&basemapid=satellite&datetime=2023-07-29T10%3A06%3A30.000Z

This is because when a user inputs the date, the code checks if the date
is in datetime list of the collection. Since Dashboard only provides
daily level selection, selected datetime gets rounded down to the
midnight of the selected date. Plume data datetimes don't have 00:00
timestamps, the selected date gets invalidated. I changed the code to
check only day-level equality to align with dashboard capacity.
fix the bug where we pass `initialPosition` prop even when there is none
defined by an editor.
Co-authored-by: Erik Escoffier <[email protected]>
Follow up of : #691
Fixing panel overflowing problem in
https://deploy-preview-201--ghg-demo.netlify.app/data-catalog/epa-ch4emission-yeargrid-v2express/explore?projection=equirectangular%7C%7C&basemapid=light&datetime=2020-01-01T00%3A00%3A00.000Z

Wow Scroll is hard. It will be ideal if we can find a way to stick the
date widget and make only layers roll, but I am giving up here.. This
change basically resurrects what `ShadowScroll` does for HTML and CSS
sans some unnecessary elements for this case.

I noticed that the window scroll sometimes pops up if you scroll very
fast on the panel area. (Sorry, I cannot reproduce stably enough to grab
a screen recording.) this error happens in production environment, too,
so this PR doesn't necessarily introduce the error. But with the default
(thicker) scroll, the error seems more noticeable.
Follow up of #692
Currently panel body is showing up scrolls even when not needed. This PR
fixes the issue.
Handles one of warnings in #693
Copy link
Collaborator

@danielfdsilva danielfdsilva left a comment

Choose a reason for hiding this comment

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

I pushed a commit with some css fixes. Mostly it rounds the button corners on the analysis page given that it was clashing with the remaining buttons on the same page. If the idea is to move to square buttons, then a overall change is required.

I'd like to us to understand if the ShadowScrollbar really is a source of problems and performance issues, given that it adds a shinier finish to the app.

Without it, the dropdowns look a bit like an afterthought.
image

Besides this I didn't see anything that would be GHG specific.

@hanbyul-here
Copy link
Collaborator

That is a good point about how the dropdown looks, so I added overscroll-behaviour: none to give a better look. I'd like to know the reason why ShadowScroll was making so many commits, too. But in any case, it is ideal not to depend on Javascript for scroll styling since it is prone to over-rendering.

@danielfdsilva
Copy link
Collaborator

@hanbyul-here My comment was related to styling, rather than behavior.
I'm not sure why it was causing so many commits just on open. That's definitely worth investigating.
On usage, it does some computation to display the shadows but that should not happen when it is opened.

Unfortunately there's no other way to style a scrolling element besides javascript. In this case we also want the shadow indicating that there's scrollable content which is not doable with css alone.
I think it is ok to degrade the visual experience a bit in favor of performance and not having errors, but I'd just lie us to be sure that this is actually causing problems.

@hanbyul-here
Copy link
Collaborator

hanbyul-here commented Oct 11, 2023

As far as I can read, ShadowScrollbar attaches extra html to generate the shadow gradient in the background. If we think it is necessary, we can still make that effect with additional markup and styles, at least for the ones that will likely have scrolls. It won't be as sleek as the ShadowScroll, but there are some ways that we can style scrolls so they look like more thoughts went into them. (I made the style change, then I thought it was better to apply it more globally, so I reverted the style.)

But all in all, you agree that the visual degradation is acceptable, then can you clarify what changes you are requesting?

@danielfdsilva
Copy link
Collaborator

I just wanted us to understand if ShadowScrollbar really is the culprit behind the errors that we were seeing and why that was happening.
Given that its use gives the product a shinier finish I'd like us to be sure of any incompatibility before removing it completely.
I think that the extra elements that it adds and the calculations it performs are a perfectly acceptable thing to have - unless it really is breaking the app.

However I do understand that there may not be time to track down this error, and this (together with removing the redirects), seems to solve the problem. I'll leave this to your decision.

@hanbyul-here hanbyul-here marked this pull request as ready for review October 12, 2023 14:57
@danielfdsilva danielfdsilva self-requested a review October 12, 2023 15:13
@hanbyul-here
Copy link
Collaborator

I will merge this, and still have a discussion about how we can improve style of scroll. nudging @faustoperez

@hanbyul-here hanbyul-here merged commit a52e077 into main Oct 12, 2023
7 checks passed
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.

5 participants