-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
use a tag instead of abbr tag for search-choice-close #2726
base: main
Are you sure you want to change the base?
Conversation
@mlettini do you have anything to say about this? |
|
coffee/chosen.proto.coffee
Outdated
@selected_item.down("span").insert { after: "<abbr class=\"search-choice-close\"></abbr>" } unless @selected_item.down("abbr") | ||
@selected_item.addClassName("chosen-single-with-deselect") | ||
unless @selected_item.down('.search-choice-close') | ||
@selected_item.down('span').insert { after: '<a class="search-choice-close"></a>' } |
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.
as there is no href, this won't get a pointer cursor in a cross-browser way (and browsers not showing it are perfectly fine according to the spec)
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.
Lines 28-30 of sass/chosen.scss
force a cursor to all links within .chosen-container
, but I agree that no href is less semantic.
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.
proper semantic would probably be a <button type="button">
rather than a <a>
btw.
Updated per @stof's comment about using a button. Removed some duplicate CSS in the process. |
fixes #495