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

report: use fixed position for hidden radios #15181

Merged
merged 3 commits into from
Jun 21, 2023
Merged

report: use fixed position for hidden radios #15181

merged 3 commits into from
Jun 21, 2023

Conversation

adamraine
Copy link
Member

Fixes https://bugs.chromium.org/p/chromium/issues/detail?id=1439785

The absolute positioning of these radios caused the report rendering to bug out. Would be nice if there was a less hacky way to hide visually but still allow screen readers to use.

@adamraine adamraine requested a review from a team as a code owner June 16, 2023 22:39
@adamraine adamraine requested review from connorjclark and removed request for a team June 16, 2023 22:39
@connorjclark
Copy link
Collaborator

Do we know why this happens?

Should we try to cherrypick the fix to 116 / 115?

@adamraine
Copy link
Member Author

Do we know why this happens?

Talked offline about this. Still not 100% sure on the root cause and may need to get help from DT rendering folks since this only happens within DT.

@paulirish
Copy link
Member

Would be nice if there was a less hacky way to hide visually but still allow screen readers to use.

The typical approach is this fairly nasty looking clip technique. Nasty looking but.. it's used everrrrrywhere:

https://github.com/twbs/bootstrap/blob/v5.0.2/scss/mixins/_visually-hidden.scss#L3-L18
https://www.a11yproject.com/posts/how-to-hide-content/
https://github.com/csstools/sanitize.css/blob/092d0d85922bfa72d28e9e8d25d80a5437c8df44/sanitize.css#L344-L356
https://css-tricks.com/inclusively-hidden/

@adamraine
Copy link
Member Author

The typical approach is this fairly nasty looking clip technique. Nasty looking but.. it's used everrrrrywhere:

I took a look at similar solutions, however it was the position: absolute not the left: -9999 which was causing issues.

copybara-service bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this pull request Jun 22, 2023
The affected code is minified and difficult to review, but the only
change made was switching the position of some elements to `fixed`:
GoogleChrome/lighthouse#15181

It is still unkown why `absolute` positioning caused this bug, but this
CL should fix the symptoms until the root cause is found.

Bug: 1439785
Change-Id: I04caee5e78d4c8257e4e4152e995f2c448fd0c34
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4633481
Commit-Queue: Connor Clark <[email protected]>
Commit-Queue: Adam Raine <[email protected]>
Reviewed-by: Connor Clark <[email protected]>
copybara-service bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this pull request Jul 12, 2023
The affected code is minified and difficult to review, but the only
change made was switching the position of some elements to `fixed`:
GoogleChrome/lighthouse#15181

It is still unkown why `absolute` positioning caused this bug, but this
CL should fix the symptoms until the root cause is found.

Bug: 1439785
Change-Id: I04caee5e78d4c8257e4e4152e995f2c448fd0c34
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4633481
Commit-Queue: Connor Clark <[email protected]>
Commit-Queue: Adam Raine <[email protected]>
Reviewed-by: Connor Clark <[email protected]>
(cherry picked from commit e4c1157)
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4681007
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.

4 participants