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

Dialog becomes off-center when browser window is resized #1789

Closed
ajmiam opened this issue Jan 19, 2024 · 10 comments · Fixed by #1795
Closed

Dialog becomes off-center when browser window is resized #1789

ajmiam opened this issue Jan 19, 2024 · 10 comments · Fixed by #1795
Assignees
Labels
bug Something isn't working

Comments

@ajmiam
Copy link

ajmiam commented Jan 19, 2024

Describe the bug (actual behavior)

Using v2's Dialog component (not full-page), when I resize the browser window, the dialog becomes off-center and sometimes partially moves off the top or left side of the screen. It's especially noticeable with horizontal resizing, but also subtly occurs with vertical resizing.

Normal appearance:
image

Appearance after resizing window:
image

Expected behavior

The dialog should stay centered, with about the same proportion of space on each of the 4 sides as it had when it was initially displayed.

Reproduction

Link to a minimal repro: Example

Note: You'll need to click "Open in new tab" and then "Link to project" to get the preview into its own window so you can better see the effects of resizing.

image

Steps to reproduce
  1. Create a Dialog component with some content
  2. View the page in the browser
  3. Press the Restore/Maximize button in the browser, and/or drag the window resizing handles horizontally and/or vertically to see the dialog box become off-centered

Additional information

An inline transform style with specific pixel translations automatically gets added to the component, in addition to the transform style (with percent translations) already set for its class in CSS. If I disable the inline style and keep the style in CSS, the dialog stays centered after a resize, as expected: (Here, the "good" style is circled in green, and the "bad" style is circled in red.)

image

@ajmiam ajmiam added the bug Something isn't working label Jan 19, 2024
@r100-stack
Copy link
Member

Thanks for the issue. Another user had reported this recently. When we investigated it a short while back, we realized that the issue might have started since #1693. We will investigate this further.

@mayank99
Copy link
Contributor

This also happens in v3. #1693 was done to fix #1551, which we get a lot more complaints about.

(Thinking out loud here) The proper fix to both issues might involve reworking the CSS to avoid percentages, so that we don't need any rounding in JS. We were talking about refactoring the dialog to use flex positioning in v3, which might avoid the issue altogether. #1770 is another related issue with these rounded transforms.

I think we can look into this after we are done with our current tasks. If the fix is simple enough, we'll backport it to v2.

@ajmiam
Copy link
Author

ajmiam commented Jan 19, 2024

Ah, I do see the blurriness problem when I disable the inline transform. It doesn't affect label text, but it does affect text values inside inputs and text in a table.

It seems like the root problem of this particular issue is that the inline transform style (which I assume is the rounded value) doesn't get updated as the window resizes. Adding a listener for that might be enough to fix this for modals that are laid out relative to the whole window. However, if a modal can be laid out relative to a chosen containing element and not the whole window, then reacting to a resize of that container might be trickier if it can change size for reasons other than the whole window being resized....

Using flex styles does seem like a good long-term plan.

@Juliakas
Copy link
Contributor

We are also experiencing this issue, but in addition to that - whenever the size of the Modal changes (lets say table data is fetched so it stretched), it doesn't get centered properly as inline transform: translate does not get updated. We worked around it and overrided it with

{
    top: 50%;
    left: 50%;
    transform: translate(-50%, -50%) !important;
}

but then the blurry text issue persists, so I am currently trying to work around and use flex layouts for such cases. Is there any good reason not to use flex based centering for dialogs?

@mayank99
Copy link
Contributor

@Juliakas You should probably set a fixed height on your table so that it doesn't grow when data is fetched. Generally it's good to minimize content shift.

The intended centering is supposed to be not in the middle of the screen but slightly above it, which becomes complicated with flex. We are looking into it though.

@mayank99
Copy link
Contributor

I'm thinking of reverting #1693. I was looking into #1551 again, and found that setting overflow: visible on the dialog element might be enough to fix the blurring; no rounded transform needed.

I will need some help confirming this though. Could you leave a 👍 on this comment if this works in your application? (or 👎 if it doesn't). Try just adding overflow: visible as an inline style on the dialog element and unchecking the rounded inline transform style.

image

/cc @ajmiam @Juliakas @AAugustBentley @HaveSpacesuit @mattl-bentley


For more context, the overflow: hidden was added mainly to avoid content floating outside when the dialog is resized to be too small. This isn't a concern for non-resizable dialogs, and also shouldn't be a problem when using Dialog.Content (or ModalContent), which is a requirement in v3 anyway.

@mayank99 mayank99 changed the title v2: Dialog becomes off-center when browser window is resized Dialog becomes off-center when browser window is resized Jan 23, 2024
@mathieu-fournier
Copy link

@Juliakas You should probably set a fixed height on your table so that it doesn't grow when data is fetched. Generally it's good to minimize content shift.

The intended centering is supposed to be not in the middle of the screen but slightly above it, which becomes complicated with flex. We are looking into it though.

To move the dialog up just a bit, we could put a empty div with some height below the dialog in the flex-column. ?

@mayank99
Copy link
Contributor

In #1795, we have removed overflow: hidden (fixing #1551) and the inline transform (fixing this issue). It's now released in 2.12.21 and 3.3.1

@Juliakas
Copy link
Contributor

@Juliakas You should probably set a fixed height on your table so that it doesn't grow when data is fetched. Generally it's good to minimize content shift.

The intended centering is supposed to be not in the middle of the screen but slightly above it, which becomes complicated with flex. We are looking into it though.

Generally yes, we should be using fixed height tables, there are some rare cases for us where jumping UI is unavoidable.

And new version solves both of our issues, we still have to make some styling changes to fully adopt it, but thanks for fixing this :)

@ajmiam
Copy link
Author

ajmiam commented Jan 30, 2024

Thank you! 2.12.21 fixed the resizing problem for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants