iTwin / iTwinUI-react

A react component library for iTwinUI.
https://github.com/iTwin/iTwinUI
Other
84 stars 23 forks source link

chore(Dialog): DialogBackdrop uses `.iui-backdrop-fixed` instead of `position: fixed` #866

Closed r100-stack closed 2 years ago

r100-stack commented 2 years ago

[CONTINUES] #860 (closed as it was pointing to v2 and not itwinui-css-v1)

Checklist

r100-stack commented 2 years ago

I think the Dialog example Modal should have the class name 'iui-backdrop-fixed' applied to the backdrop instead of position: fixed, but inspecting the code doesn't seem to show that? image

When confirming on my end, I think .iui-backdrop-fixed is applied to the backdrop and the inline position:fixed is not there. To confirm, are you running the code in the rohan/v1-dialog-position-fixed-1 branch?

image

elephantcatdog commented 2 years ago

When confirming on my end, I think .iui-backdrop-fixed is applied to the backdrop and the inline position:fixed is not there. To confirm, are you running the code in the rohan/v1-dialog-position-fixed-1 branch?

Aha, I was on the previous one.

r100-stack commented 2 years ago

Line 91 position: relativeTo === 'container' ? 'absolute' : 'fixed', seems like it should be removed from Dialog.tsx?

Well, I've been playing with it and am not 100% on how it works in react as it doesn't seem to be a 1-to-1 to the css code. Having position: absolute/fixed seems necessary for the dialog wrapper in react even though it's only on the backdrop in CSS.

I just checked it, looks like that's true. Looks like in CSS, .iui-dialog-wrapper has display: relative but in react, that is overridden by an inline style position: fixed.

Removing this inline style in React causes the dialog and the backdrop to disappear. Setting .iui-dialog-wrapper in CSS with display: fixed inline style expands to the whole screen, like below:

vokoscreenNG-2022-10-11_17-23-25.webm

Thus, I'm not sure if there can be a 1-1 mapping. But this might be something to look into in another issue/PR. @elephantcatdog @gretanausedaite @mayank99 What do you think?

r100-stack commented 2 years ago

Thus, I'm not sure if there can be a 1-1 mapping. But this might be something to look into in another issue/PR.

I just realized, Mayank had a CSS PR #773 to change the position of .iui-dialog-wrapper from relative to fixed. There might be a 1-1 mapping after implementing those changes in react.

mayank99 commented 2 years ago

Yes, I left this comment above:

There are additional changes made in iTwin/iTwinUI#773 but that can come in a separate PR.

That PR is available in @itwin/itwinui-css@1.0.0-dev.7.