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

Some websites (like GitHub) appear flat when their DOM clearly isn't #10

Open
OrionReed opened this issue Mar 29, 2024 · 7 comments · Fixed by #13
Open

Some websites (like GitHub) appear flat when their DOM clearly isn't #10

OrionReed opened this issue Mar 29, 2024 · 7 comments · Fixed by #13
Labels
bug Something isn't working

Comments

@OrionReed
Copy link
Owner

This one is self explanatory, some DOMs are entirely flat when they should not be. There is probably something in the body styling or elsewhere that effects all downstream styling changes, but I'm not sure what.
Screenshot 2024-03-28 at 22 50 41

@lucasmoes
Copy link

same for gmail

@krzentner
Copy link
Contributor

krzentner commented Mar 29, 2024

Forcing isolation: "auto", appears to fix this issue on GitHub. I'm not completely sure what the consequences of that will be, but I don't observe any large layout changes. It does not seem to be sufficient to fix gmail (although perhaps necessary).

@OrionReed
Copy link
Owner Author

OrionReed commented Mar 29, 2024

^ whoops, I misclicked there, not done yet!

isolation: "auto" is a great improvement!
Screenshot 2024-03-29 at 03 15 19

I also discovered it's easier to see some of the issues when the transform is animated so I added a default transition of 2.5s

@OrionReed OrionReed added the bug Something isn't working label Mar 29, 2024
@krzentner
Copy link
Contributor

krzentner commented Mar 29, 2024

Looking into why isolation: "auto" fixes some pages lead me to the CSS Transforms Level 2 Spec, which lists a number of attributes that "require the user agent to create a flattened representation of the descendant elements before they can be applied, and therefore force the element to have a used style of flat for preserve-3d.":

overflow: any value other than visible or clip.
opacity: any value less than 1.
filter: any value other than none.
clip: any value other than auto.
clip-path: any value other than none.
isolation: used value of isolate.
mask-image: any value other than none.
mask-border-source: any value other than none.
mix-blend-mode: any value other than normal.
contain: paint and any other property/value combination that causes paint containment. Note: this includes any property that affect the used value of the contain property, such as content-visibility: hidden.

Most pages affected by this issue are probably due to one of these attributes on one of the top-most elements of the page. Forcing most of these values shouldn't have too surprising of an effect (we're not trying to keep the exact same page layout anyways), but overflow might be a little troublesome.

@krzentner
Copy link
Contributor

In gmail's case, it appears to be because overflow-y: hidden !important is set on the body via a rule, which overrides the overflow: visible we add to the element directly.

@OrionReed
Copy link
Owner Author

That's great info! Not entirely what "require the user agent to create a flattened representation" is referring to in the spec, I'll have to go digging. I think this approach of actually looking at the spec for each issue point is the only way this gets to be ~100% reliable.

I found that forcing visibility: "visible" on all nodes is probably also a good idea. It fills in all (?) of the gaps I've seen in the DOM where an element is N levels higher but with nothing in between. It's part of the CSS Display Module Level 3 spec.

I agree that exact same layout is unnecessary but it would be good to get as close as possible, in some cases this layout change can be quite extreme such as in #14 — it may be a cause of other rendering issues too which we can't easily see while the layout changes unpredictably.

@OrionReed
Copy link
Owner Author

Adding your comment from the Force isolation: auto PR as it had some good insights:

Thanks! Unfortunately, I'm also not completely sure why this works, to be honest. The only element with isolation: "isolate" set on the github main page is the div directly inside the body. The only affect of that attribute according to MDN is to force the creation of a stacking context. However, we're also setting transform to a non-none value on this element, and that should already be creating a stacking context. It's also not clear to me why isolation: "isolate" on an ancestor element affects transforms on all descendant elements.

Edit: I figured it out. This isn't due to stacking contexts at all, but due to CSS Transforms explicitly looking for the isolation value.

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.

3 participants