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

feat(inpage): Using the right element to determine the top position in mobile - FRONT-4620 #3629

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

planctus
Copy link
Collaborator

No description provided.

Copy link

github-actions bot commented Sep 16, 2024

@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 07:02 Inactive
Copy link
Contributor

@emeryro emeryro left a comment

Choose a reason for hiding this comment

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

I'm a little confused here. By checking the code, it seems that this topPosition is in fact used to calculate the max height (for the scrolling).
So it subtracted 16px on mobile, which correspond to the space that we have at the bottom
image
With the proposed fix, this space is reduced to 12px on EC, and no space at all on EU (there is no padding on the wrapper)
image
I don't recall if this space was requested by the designers

@planctus
Copy link
Collaborator Author

i don't recall designers having ever considered the presence of scrollbars. :) The difference here was due to the negative margin set on the wrapper in EC, now we take also that into account.

To explain a bit, before we were using the button to get this "top position", and then we were manually "compensating" this with an harcoded value, now we get the wrapper position and it can be used as such, just that there is this little overhead of the negative margin for EC which introduced this misalignment. Currently there will be no spacing under the dropdown in EC and EU when the scrollbar will appear.

@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 09:21 Inactive
@emeryro emeryro merged commit 8b2164c into v4-dev Sep 16, 2024
7 checks passed
@emeryro emeryro deleted the FRONT-4620-Inpage-trigger-bottom branch September 16, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants