fix(styles): stop fade-in from permanently making elements a containing block - #23
Merged
Conversation
This file contains hidden or 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
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 trap
The final keyframe ends at
translateY(0), andfill-mode: bothretains the last keyframe's value forever.translateY(0)is still a transform — notnone— so every.animate-fade-inelement stays a containing block forposition: fixeddescendants permanently, long after the 0.3s animation is over.Anything
fixedrendered inside such a wrapper is then positioned against the wrapper instead of the viewport.How it surfaced
In the dev portal, a
fixed inset-0dialog rendered inside a page whose root isanimate-fade-in. On a tall page, centring inside the wrapper happened to land near the middle of the screen and looked correct. On a short page the dialog centred a few hundred pixels down and its top went above the viewport, unreachable by scrolling — a checklist with no heading and no way to get to it.It read as a bug in the page that happened to be short. It was this rule.
The fix
End the animation at
transform: none. It interpolates identically —noneis treated as the identity transform — so the animation looks exactly the same, and no containing block is left behind. The reason is written into the file so the next person does not "tidy" it back totranslateY(0).Consumers get this for free once they pick up the release. The dev portal has already been fixed at its own layer by rendering dialogs through a portal (sphere-dev-portal#37), which is the right defence regardless of this rule — this change removes the trap for everything else.
Verification
109 tests pass,
tsupbuild clean,tsc --noEmitsilent.npm run lintcannot run in this repo — eslint is not installed — which is pre-existing and unrelated to this change.