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
Only use hidden until-found in panels if browsers supports it #8962
Only use hidden until-found in panels if browsers supports it #8962
Conversation
lb-
commented
Aug 7, 2022
- fixes hidden="until-found" usage broken in browsers lacking support #8961
- Note: There may be a different fix for this (CSS only) but this seemed the quickest way to get to the goal of the panels closing / opening correctly by using feature detection for this + as browsers upgrade they will automatically get the new behaviour.
- Tested on Chrome, Firefox and Safari - all now correctly collapse the panels, but Chrome still has the ability to search in the page to find content within collapsed items.
@gasman as Thibaud may be away this week I was not sure if he would get a chance to do a fix so I just did my best fix for now I could think of so the RC is not blocked. We can maybe find a different fix during the RC process if needed. |
Manage this branch in SquashTest this branch here: https://fix8961-use-hidden-until-found-961bk.squash.io |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy with this fix - works well in Firefox and is consistent with the fallback suggested on https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden . (It strikes me as a little odd that caniuse.com links to https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden as the canonical reference, but that page doesn't actually explain until-found
at all - ah well!)
Will hold off merging until tomorrow in case @thibaudcolas has any strong feelings against it, but if not I think this is good to go.
Thank you @lb-! I’ll review this later today. I would have thought it would be the styling that’s off rather than the JS logic for this, but if I got this wrong then we can merge this as-is. |
@thibaudcolas yeah I agree there is probably a styling solution for this but when I looked at it - it was not immediately obvious. Hence, I went with the recommended approach from the chrome dev docs for feature detection instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging this as-is as I don’t think I’ll have the time to review this properly!
Thanks Thibaud |
I just noticed that exact issue (reviewing this code as I'm going to be doing some work on a global expand / contract button) - I've reported it on the MDN repo now. |