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

fix: Modal component is shown too high and gets out of viewport #7466

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dgolovin
Copy link
Contributor

@dgolovin dgolovin commented Jun 5, 2024

What does this PR do?

Removes absolute position for Modal component because it is already has w-full and h-full

Screenshot / video of UI

That will change this behavior

image

to a normal one

image

What issues does this PR fix or reference?

How to test this PR?

  • Tests are covering the bug fix or the new feature

@dgolovin dgolovin requested review from benoitf and a team as code owners June 5, 2024 05:52
@dgolovin dgolovin requested review from axel7083 and lstocchi and removed request for a team June 5, 2024 05:52
@dgolovin dgolovin changed the title fix: Modal component shown is too high and gets out of viewport fix: Modal component is shown too high and gets out of viewport Jun 5, 2024
@axel7083
Copy link
Contributor

axel7083 commented Jun 5, 2024

cc @deboer-tim

@axel7083 axel7083 requested a review from deboer-tim June 5, 2024 08:01
@dgolovin dgolovin force-pushed the fix-modal-position-for-message-box branch from a392f98 to ee92594 Compare June 6, 2024 03:34
Copy link
Contributor

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

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

It introduces some issue in some cases.

Like when you click on the store name (troubleshooting -> stores -> click on a store name)
image

or if you try to push an image (images -> push image)
image

@deboer-tim
Copy link
Collaborator

Took a quick look. If anything I think we need to keep inset-0 on both the outer div and button (replacing top/left/w-*). I think we need to go back to compare against previous release to see how adopting Modal affected this - there's clearly a bigger issue here where we'll need to use z-ordering instead of overflow-hidden, but might be easier in the meantime to use a workaround like class:mt-[40px]="{top}"?

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.

None yet

4 participants