Skip to content

possible fix for occluded height #449

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

Merged
merged 2 commits into from
May 20, 2025
Merged

possible fix for occluded height #449

merged 2 commits into from
May 20, 2025

Conversation

joshreisner
Copy link
Contributor

@joshreisner joshreisner commented May 18, 2025

ideally, TSML UI will at a minimum fill the available space on the screen

when it's in list view, it typically scrolls higher than the viewport and that's fine

when in map view however, it can sometimes be too small or too big. a recent PR set the min-height to be 100dvh, which is great except when there's a fixed element, such as a header, occluding part of the viewport

check out https://aasanjose.org/meetings?view=map - ideally the map would not go off the bottom of the screen

this PR computes the height of probably-occluding elements and adjusts TSML UI's min-height accordingly. also adds example test with a fixed header

concerns:

  • calclulating the height of fixed elements is a little expensive. this PR mitigates this by 'debouncing' the checks until .25s after a user has stopped resizing / scrolling. but is there a better way to do this?
Screenshot 2025-05-18 at 9 52 14 AM

@joshreisner joshreisner requested a review from gkovats May 18, 2025 16:54
@joshreisner joshreisner self-assigned this May 18, 2025
Copy link

netlify bot commented May 18, 2025

Deploy Preview for tsml-ui ready!

Name Link
🔨 Latest commit 8790c34
🔍 Latest deploy log https://app.netlify.com/projects/tsml-ui/deploys/682a435d34b7c80008acfd90
😎 Deploy Preview https://deploy-preview-449--tsml-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 project configuration.

) : (
<div style={{ display: 'flex', flexGrow: 1 }}>
<Map
<div style={{ minHeight: `calc(100dvh - ${occludedHeight}px)` }}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the diff looks a little weird below - the only change here is wrapping everything in this div ☝️

Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM, works on my machine

Copy link
Collaborator

@gkovats gkovats left a comment

Choose a reason for hiding this comment

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

Honestly it makes sense, and if it somehow clashes with oddball designs on a few sites, it's small enough to work around.

It might be good to make this change default, and add some config option to disable just in case there's some weird clash, but we can always do that later.

@joshreisner joshreisner merged commit 023451a into main May 20, 2025
4 of 5 checks passed
@joshreisner
Copy link
Contributor Author

thanks! yeah i'm open to making this a configuration, or just trying to iterate when we hear reports of issues

looks like it's working pretty well so far (in mac safari at least) on

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.

2 participants