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

AnimateHeight fixes for calculating the node height #2910

Merged
merged 5 commits into from
May 6, 2020
Merged

AnimateHeight fixes for calculating the node height #2910

merged 5 commits into from
May 6, 2020

Conversation

tomasztunik
Copy link
Contributor

@tomasztunik tomasztunik commented May 6, 2020

This fixes an issue with reading the height of the AnimateHeight node. Before scrollHeight was used which by default rounds the height value down to an integer (mentioned here @MDN) which sometimes for certain OS/browser/zoom-level/font combinations could cause the scrollbar to be visible in the filter add/edit popouts if the height of the node was not an integer.

Recommended at MDN getBoundingClientRect().height was not providing correct value reliably for the initial state of the node and only offsetHeight did work correctly in my tests for each case.

Also provided proper value none for no transition CSS transition property.

scroll-visible

@changeset-bot
Copy link

changeset-bot bot commented May 6, 2020

🦋 Changeset is good to go

Latest commit: 3aa1d6e

We got this.

This PR includes changesets to release 1 package
Name Type
@keystonejs/app-admin-ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@timleslie timleslie left a comment

Choose a reason for hiding this comment

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

Great catch, thanks for the PR and detailed explanation 👍

@timleslie timleslie merged commit 6c19f04 into keystonejs:master May 6, 2020
@github-actions github-actions bot mentioned this pull request May 6, 2020
@tomasztunik tomasztunik deleted the app-admin-ui/animate-height-fixes branch May 8, 2020 19:41
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