#27555 closed defect (bug) (fixed)
Make tag post meta box more accessible
Reported by: | joedolson | Owned by: | joedolson |
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Editor | Keywords: | has-patch commit |
Focuses: | accessibility | Cc: |
Description
The tag post meta box has a couple of issues:
1) Current tags are not keyboard-navigable, so can't be deleted.
2) If a tag is added from quicklinks, no confirmation of adding is read.
This patch makes the tag delete link focusable, by adding an href attribute value; adds a :focus state equivalent to the existing :hover state for keyboard users; adds aria-live=assertive to the container where tags are injected, and replaces the 'X' currently used for the link text with off-screen text stating "Delete [tag name]".
Known issues with this patch:
- Adding the href value causes the usual moving window problem; I couldn't find where to attach the default event prevention in post.js
- 'Delete val' is not translatable as is; do we use wp_localize_script with core? Not sure what to do with that.
Attachments (9)
Change History (46)
#3
@
11 years ago
So, there may just be something I'm not understanding, but why was this changed to version 3.0? I'm not sure I get the logic.
#4
@
11 years ago
The 'Version' field indicates the first version in which the issue was present (unlike the 'Milestone' field which indicates the target version for getting it fixed).
In this particular case the first affected version isn't really relevant (unlike more severe bugs when we try to identify exactly when they were introduced) but we do it anyway as part of the bug management workflow.
#5
@
11 years ago
Interesting. Good to know! I've always wondered what that represented; is there an easier way to track that back other than to just start browsing tags? I can start adding that info when I create reports; but it would be nice to do it somehow other than totally manually.
#6
@
10 years ago
Refreshed patch:
- adds e.preventDefault()
- makes "Delete" translatable
- rebuilt from the SVN install root
Main outstanding issues:
- after a new tag is added, focus should not be moved back to the input field
- in some circumstances, screen readers may read out something like "Delete tagname tagname" (2 times)
@joedolson, Accessibility Team, anyone: maybe we should avoid to introduce new non-links with href="#"
, what about buttons instead? Will end up with too many buttons?
#7
follow-up:
↓ 8
@
10 years ago
What would feelings be about holding on this (along with other changes to the tags metabox) for a release and seeing about a switch to a different JS library for the UI? I would love to use Select2 for it instead, assuming we'd bring it in 4.2 for some other things we're working on in the admin: http://ivaynberg.github.io/select2/select2-latest.html#tags
If Select2 needs upstream fixes for a11y, would also love to know about that.
#8
in reply to:
↑ 7
@
10 years ago
Replying to helen:
If Select2 needs upstream fixes for a11y, would also love to know about that.
Just started looking into Select2, where should we report accessibility considerations for the dropdown replacement and for the tagging thing? they're 2 separate cases. New tickets, github, upstream?
#9
@
10 years ago
Select2 issues should probably be reported upstream to the Select2 developers; but I don't know where they handle it.
I generally agree that if we're going to make significant changes to this area in a near release, it doesn't make sense to work on the current iteration now.
This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.
8 years ago
#14
@
8 years ago
Not going to wait on Select2 issues at this point; those may eventually be resolved, but this is simple enough that we can improve it significantly now without continuing to wait.
@afercia - Yes, I agree, probably going with a button; possibly using text-styling will be a better choice.
Will get this patch refreshed soon.
#15
@
8 years ago
Refreshed the 27555.2 patch so that it applies properly to the current source and attached it to this ticket.
This ticket was mentioned in Slack in #accessibility by mantismamita. View the logs.
8 years ago
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
8 years ago
#18
@
8 years ago
- Keywords early added
- Milestone changed from 4.6 to Future Release
We're almost in 4.6 beta 3, sorry to punt this to future release but definitely something that should be done early in the next release cycle.
This ticket was mentioned in Slack in #core by desrosj. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
#22
@
8 years ago
- Keywords needs-testing dev-feedback removed
Refreshed patch. Please notice the tags box should be tested also in Press This.
- the remove tag links are now buttons
- they use the circular focus style (see screenshot below)
- they use a screen reader text "Remove item:" + tagname
wp.a11y.speak()
is used to give feedback to screen reader users when adding/removing tags
Note: tags can be also custom taxonomies terms so the strings can't refer to "tags" but they should use a generic wording. Alternatively, new taxonomy labels should be introduced and there should be a way to pass them to the JS part. I've opted to keep things simple and use "item". Any feedback welcome.
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
8 years ago
#24
@
8 years ago
- Keywords commit added
Refreshed patch. As per today's discussion during the accessibility weekly meeting on Slack, changes the term "Item" used in the new strings in "Term". Also, some renaming to better reflect the terms are not "deleted" and instead they're "removed". Looks good to me and any testing (also on Press This) is very welcome!
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
#28
follow-up:
↓ 29
@
8 years ago
Refreshed patch after [38797].
Replying to azaozz:
Yes, 27555.3.diff needs a (small) refresh after [38797], sorry. Also, keep in mind that everything that's needed for the Tags metaboxes is also probably needed for Quick/Bulk Edit :)
No problem :) Yep, I've considered what happens on Quick/Bulk Edit and the interaction is a bit different since the tags are "selected" but not actually "added" until the form is submitted. Also, tags removal is a manual process. So I'd say it's fine to have just the Term selected.
feedback message there.
Instead, on the Tags meta box there are specific controls and actions to add/remove (even though one could argue that terms are actually added just when the post is saved). Thus, these actions need some additional feedback.
Patch looks good to me, would appreciate a code review. Any feedback's welcome. Trying to recap what the patch does:
- changes the "X" remove tag links in buttons
- uses the circular focus style on the buttons
- adds a screen reader text "Remove item:" + tagname
- uses
wp.a11y.speak()
to give screen reader users feedback when adding/removing tags - makes the
tagcloud-link
toggle a button, with anaria-expanded
attribute to indicate the tag cloud collapsed/expanded state - changes colors for the autocomplete highlighted option in order to have a sufficient contrast ratio, doesn't touch the
wplink
colors (will open a separate ticket) - reduces the font size for the autocomplete on Press This, it was a bit too large (see screenshot)
- removes CSS related to the old
suggest.js
(.ac_results) from Press This but keeps it in core for backwards compatibility
Highlighted option accessible colors and X button circular focus:
Font size in Press This was a bit too large:
#29
in reply to:
↑ 28
@
8 years ago
Replying to afercia:
27555.4.diff looks pretty good. Tweaked tags-suggest.js a bit in 27555.5.diff and added some inline docs.
The only thing I'm wondering about is the 'circular focus style' on the X buttons. It seems the change in color from light grey to red is pretty strong indication which one is focused. If there is not enough contrast, perhaps can make the red a bit darker?
#30
@
8 years ago
Open to suggestions about colors. We should consider also the hover state, so currently the colors are:
The red is the one already used for the "dismiss" X icons, for example:
But, just realized the light grey for the normal state doesn't have a sufficient color contrast ratio for accessibility, should be at least 4.5:1. One option could be making the icons use the same blue used for links:
alternatively, they should use a darker grey.
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
#34
@
8 years ago
Refreshed patch. Discussed a bit on Slack a few days ago, looks good to go. https://wordpress.slack.com/archives/accessibility/p1476728057002403
#36
@
8 years ago
This change includes removing "return false" from the click handler on $( '.tagcloud-link' ):
Previously an anchor tag with that class could have any href and it wouldn't make a difference. Now the href must be "javascript:;" to accommodate the fact that the handler no longer returns false. Is that a bug or is there a reason it was removed?
The specific case I'm looking at is in the bp-event-organiser plugin which relies on the tags-box library:
wp_enqueue_script:
https://github.com/cuny-academic-commons/bp-event-organiser/blob/master/includes/wp-frontend-admin-screen/wp-frontend-admin-screen.php#L448
the markup which includes the class "tagcloud" with an href of "#titlediv", which the browser obeys after upgrading WP to include the new tags-box (previously the href was ignored due to "return false"):
https://github.com/cuny-academic-commons/bp-event-organiser/blob/master/includes/wp-frontend-admin-screen/abstraction-metabox.php#L476
#37
@
8 years ago
@cyclic thanks for your comment. In WordPress, that link has been changed in a button element with a type="button"
attribute and as such, it doesn't have any default action to prevent. Previously, it was an anchor tag and thus it needed a return false
to prevent the link default action.
For accessibility reasons, links that are not real links and instead perform an action should be buttons. In the last months, WordPress has changed many non-links in buttons and still there are many other ones to change. Ideally, the plugin you mentioned should follow core's best practices and change its link in a button :)
First pass at addressing issues with tag meta box