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

Focusable: Fix handling of visibility: collapse #1843

Merged
merged 1 commit into from
Oct 14, 2020
Merged

Focusable: Fix handling of visibility: collapse #1843

merged 1 commit into from
Oct 14, 2020

Conversation

PaulCapron
Copy link
Contributor

@PaulCapron PaulCapron commented Oct 31, 2017

"collapse" is similar to "hidden", with a slight difference in the case of tr/tbody/td/colgroup elements.
See https://www.w3.org/TR/CSS22/visufx.html#visibility
See https://www.w3.org/TR/CSS22/tables.html#dynamic-effects
See https://developer.mozilla.org/en-US/docs/Web/CSS/visibility#Table_example

"visibility: collapse" elements are always not focusable, though.

Commit d302596 introduced this regression by testing with !== "hidden" instead of === "visible".

"collapse" is similar to "hidden", with a slight difference in the case
of tr/tbody/td/colgroup elements.
See https://www.w3.org/TR/CSS22/visufx.html#visibility
See https://www.w3.org/TR/CSS22/tables.html#dynamic-effects
See https://developer.mozilla.org/en-US/docs/Web/CSS/visibility#Table_example

"visibility: collapse" elements are always not focusable, though.

Commit d302596 introduced a regression by testing with `!== "hidden"`
instead of `=== "visible"`.
@jsf-clabot
Copy link

jsf-clabot commented Oct 31, 2017

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@PaulCapron
Copy link
Contributor Author

Any chance this will eventually get reviewed (and, hopefully, merged)? It’s been 3 years.

@melloware
Copy link

with 41 open PR's i feel like activity on Jquery UI is very slow to get merged but 3 years is a little excessive. Hopefully this will get merged before 1.13 is released!

@mgol mgol requested review from fnagel and arschmitz October 14, 2020 15:53
Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

Copy link
Member

@fnagel fnagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 by reading

Sorry for the delay.

@fnagel fnagel added the Bug label Oct 14, 2020
@mgol mgol merged commit f5d38e2 into jquery:master Oct 14, 2020
@mgol
Copy link
Member

mgol commented Oct 14, 2020

Landed, thank you!

@PaulCapron
Copy link
Contributor Author

Great, dziękuję!

@mgol mgol added this to the 1.13 milestone Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 participants