Fix scrolling footer #1022
Merged
Fix scrolling footer #1022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Mhhh while svh seems to be supported by most browsers, this is rather recent and does not work in Opera. Would the issue also be fixed by simply replacing & > div with & > div:first-of-kind? Or somehow else annotating height: 100% to only our own div?
I wouldn't hard block svh but... if we can easily avoid bumping the browser requirement for such layout-critical thing...
|
Looking at caniuse, it seems to me that these new units are supported by almost every browser, including opera. Or did you test this with opera mini? (source: https://caniuse.com/mdn-css_types_length_viewport_percentage_units_small). But I think |
The footer was scrolling with certain browser extensions that add hidden divs to the body. We were setting all descendant divs of the body to `100%` height, which caused them to either push elements up or let users scroll beyond any visible content. The solution is to only set the height on the first descendant div.
owi92
force-pushed
the
fix-floating-footer
branch
from
7737947 to
cf202a0
Compare
LukasKalbertodt
approved these changes
Dec 6, 2023
LukasKalbertodt
added a commit
to LukasKalbertodt/tobira
that referenced
this pull request
Dec 14, 2023
Follow up to elan-ev#1022 where stupidly suggested `:first-of-kind` because suuurely, browser extensions would only add elements to the end of the `<body>` but not the beginning, riiight? Of course that's a wrong assumption. I noticed this with Bitwarden on the login page. This whole "extensions randomly modifying DOM" thing seems super brittle to me, oof.
owi92
added a commit
that referenced
this pull request
Dec 19, 2023
Follow up to #1022 where stupidly suggested `:first-of-kind` because suuurely, browser extensions would only add elements to the end of the `<body>` but not the beginning, riiight? Of course that's a wrong assumption. I noticed this with Bitwarden on the login page. This whole "extensions randomly modifying DOM" thing seems super brittle to me, oof.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The footer was scrolling with certain browser extensions that add hidden divs to the body.
We were setting all descendant divs of the body to
100%height, which caused them to either push elements up or let users scroll beyond any visible content.This is fixed by removing the child combinator and adding a height rule using the newish svh (= small viewport height) to the
<Outer>wrapper component only. Usingsvhmakes sure that the page is also correctly displayed on mobile devices (which is not the case withvh).