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

Add CSS to support scrolling in layers toolbar in the Cesium map view #2249

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

ianguerin
Copy link
Collaborator

  • Set a fixed height for body element to force map view to appear on page.
  • Add some padding to allow for scrollbar in layers toolbar.
  • Fix issue with cutting off last layer in toolbar on narrow laptop screen due to a hardcoded value for the header height.

GitHub issue: #2242

@mbjones mbjones requested a review from robyngit January 20, 2024 21:39
@mbjones mbjones added portals Anything related to portals pdg Permafrost Discovery Gateway labels Jan 20, 2024
@mbjones
Copy link
Member

mbjones commented Jan 20, 2024

Thanks @ianguerin this is great. Could you provide a screenshot in the PR so folks that don't have a dev-instance of MetacatUI set up can see what the feature will look like?

@robyngit could you review please and maybe this can be included in the release along with the hierarchical data view? Thanks.

@robyngit robyngit changed the base branch from main to develop January 22, 2024 17:31
@robyngit robyngit linked an issue Jan 22, 2024 that may be closed by this pull request
@ianguerin
Copy link
Collaborator Author

Here are some before an after screenshots:
Before the layers list was scrollable:
before being scrollable

After, with the layers list scrollable
after_scrollable

Before, when scrolled to the bottom:
before, scrolled to the bottom

After, when scrolled to the bottom
after, scrolled to the bottom

@ianguerin ianguerin marked this pull request as ready for review January 22, 2024 18:03
Copy link
Member

@robyngit robyngit left a comment

Choose a reason for hiding this comment

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

Great work, @ianguerin! Thank you for being so proactive on this and congrats on your first MetacatUI PR 🎉! Your changes look perfect for the PDG portal in the arctic theme. I appreciate that you added only the most minimal CSS rules to make this work; it helps keep our already very large CSS file from getting even larger!

Because MetacatUI is used by other projects, it's important for us to check that changes align well across various themes and contexts before merging them into the main codebase. I've outlined some of these considerations below.

In non-arctic themes

When testing in non-arctic themes, like knb, there seems to be a slight issue: the cesium map expands to accommodate this list, instead of having the list scroll as intended.

To switch to another theme, change theme: "arctic" to theme: "knb" in the config.js file.

In non-portal contexts

Repositories may also use the cesium map, instead of Google maps, in the main data search catalog. In this context, there's a bit of a display problem. The overflow: hidden CSS rule added to the .map-view__toolbar-container class is causing the right side of the layer list to be clipped off.

To see this, pull in the drp theme from the metacatui-themes repo, and configure the theme option to drp. The search catalog with layer list expanded looks like this:

Screenshot 2024-01-22 at 16 20 24

We can collaborate on fixing this issue given that other themes and contexts may be outside the scope of your PDG work. In any case, thank you again for this and I look forward to more of your contributions!

Set a fixed height for body element to force map view to appear on page.
Add some padding to allow for scrollbar in layers toolbar.
Fix issue with cutting off last layer in toolbar on narrow laptop
screen due to a hardcoded value for the header height.

GitHub issue: NCEAS#2242

temp commit with changes for PR comments
@ianguerin ianguerin force-pushed the feature-2242-layers-scrollbar branch from 17c0841 to ee2bed7 Compare January 23, 2024 01:22
@ianguerin
Copy link
Collaborator Author

Thanks for the detailed review, Robyn. I've updated the PR with changes to address the issues you found with the toolbar. It looks like the issue with the drp theme was probably broken without any scroll capability (you couldn't actually access the layers in the list below the visible content), so this change should help.

Copy link
Member

@robyngit robyngit left a comment

Choose a reason for hiding this comment

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

Nice work @ianguerin!!

@robyngit robyngit added this to the 2.28.0 milestone Jan 25, 2024
@robyngit robyngit merged commit 58b4a3e into NCEAS:develop Jan 25, 2024
1 check passed
@robyngit robyngit removed this from the 2.28.0 milestone Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pdg Permafrost Discovery Gateway portals Anything related to portals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add scroll bar to the cesium layer list
4 participants