Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 5 years ago

#25459 closed enhancement (fixed)

Provide more meaningful links in Edit Post/Page

Reported by: grahamarmfield's profile grahamarmfield Owned by: johnbillion's profile johnbillion
Milestone: 3.9 Priority: low
Severity: minor Version: 3.6.1
Component: Posts, Post Types Keywords: has-patch
Focuses: accessibility, administration Cc:

Description

In the main Edit Page/Post screen of the admin area, there are links within the Publish box that are not sufficiently meaningful for users of screen readers.

The links are:

  • To edit the status
  • To edit the visibility
  • To browse the revisions
  • To edit the published date

Currently the links are all one word - Edit, Edit, Browse, Edit.

Extra text should be added for screen reader users to provide context for these links. The links ideally should be:

  • Edit status
  • Edit visibility
  • Browse revisions
  • Edit publish date

It is acceptable that the extra text in the links is wrapped in the visually-hidden class to it is only accessible by screen readers.

Attachments (9)

publish-links.jpg (19.2 KB) - added by grahamarmfield 10 years ago.
Screenshot showing links that I'm talking about.
25459.1.diff (2.5 KB) - added by atimmer 10 years ago.
25459.2.diff (2.5 KB) - added by SergeyBiryukov 10 years ago.
25459-functions.patch (1.8 KB) - added by grahamarmfield 10 years ago.
Patch to add two new functions for use when providing short-form and long-form text.
25459-meta-boxes.patch (2.4 KB) - added by grahamarmfield 10 years ago.
The patch for the meta boxes using the functions provided by the other patch
25459.diff (1.2 KB) - added by neil_pie 10 years ago.
25459.3.diff (1.2 KB) - added by GrahamArmfield 10 years ago.
New patch which combines Neil Pie's functionality with a more concise function name.
25459.4.diff (3.5 KB) - added by johnbillion 10 years ago.
25459.5.diff (2.7 KB) - added by johnbillion 10 years ago.

Download all attachments as: .zip

Change History (73)

@grahamarmfield
10 years ago

Screenshot showing links that I'm talking about.

#1 @toscho
10 years ago

  • Cc info@… added

For the revisions, I think we could just make the text 15 Revisions clickable.

#2 follow-up: @azaozz
10 years ago

What would be the best way to do this without changing the linked text? Would an WAI-ARIA attribute work well (something like aria-label or aria-labelledby)?

#3 @grahamarmfield
10 years ago

Please ignore my mention of visually-hidden class - the correct WordPress class is screen-reader-text.

#4 in reply to: ↑ 2 @grahamarmfield
10 years ago

Replying to azaozz:

What would be the best way to do this without changing the linked text? Would an WAI-ARIA attribute work well (something like aria-label or aria-labelledby)?

We'd have to test that, but I don't think screen readers are likely to pull in the aria-label when listing links for user.

What I had in mind would be to do:

<a href="...">Edit<span class="screen-reader-text"> status</span></a>

The experience would not change for sighted users but screen readers get the full links - both when it gets focus, and also when screen reader users use the List Links functionality.

#5 follow-ups: @toscho
10 years ago

A longer visible link text could help sighted users too: easier to hit.

#6 in reply to: ↑ 5 @grahamarmfield
10 years ago

Replying to toscho:

A longer visible link text could help sighted users too: easier to hit.

True - especially on touch device.

#7 @rianrietveld
10 years ago

So, our first #wpa11y comment on the WordCamp Europe contributors day :-)
After a long discussion between people with different languages we concluded that a good solution might be the next one, because in different languages the word Edit link will change with the action. So adding the action n brackets behind the word Edit and visually hiding it would be a good compromise.

So:
Add inside the link a hidden text for screen readers

in wp-admin/includes/meta-boxes.php change for the status:

<a href="#post_status" <?php if ( 'private' == $post->post_status ) { ?>style="display:none;" <?php } ?>class="edit-post-status hide-if-no-js"><?php _e('Edit') ?></a>

change it into:

<a href="#post_status" <?php if ( 'private' == $post->post_status ) { ?>style="display:none;" <?php } ?>class="edit-post-status hide-if-no-js"><?php _e('Edit') ?><span class="screen-reader-text"> (<?php _e('Status') ?>)</span></a>

for visuality

<a href="#visibility" class="edit-visibility hide-if-no-js"><?php _e('Edit'); ?></a>

change it into:

<a href="#visibility" class="edit-visibility hide-if-no-js"><?php _e('Edit'); ?><span class="screen-reader-text"> (<?php _e('Visibility'); ?>)</span></a>

An extra entry in the pot-file has to be made for Visibility without the :

for date published

<a href="#edit_timestamp" class="edit-timestamp hide-if-no-js" tabindex='4'><?php _e('Edit') ?></a>

change it into:

<a href="#edit_timestamp" class="edit-timestamp hide-if-no-js" tabindex='4'><?php _e('Edit') ?><span class="screen-reader-text"> (<?php _e('Date'); ?>)</span></a>

In this way the construction will be valid for all languages, the visual output will be the same and one extra line will have to be added to the translation files.

Last edited 10 years ago by rianrietveld (previous) (diff)

@atimmer
10 years ago

#8 @atimmer
10 years ago

25459.1.diff is the patch for this. It adds the screen reader text to links.

#9 in reply to: ↑ 5 ; follow-up: @SergeyBiryukov
10 years ago

Replying to toscho:

A longer visible link text could help sighted users too: easier to hit.

It would also help i18n.

25459.1.diff isn't really localizable as is, since it's just separate words rather than a string that could be translated into meaningful text. At least a context should be added for the new words, so that a proper grammatical case could be used.

If a longer visible text would seem redundant, could we add the verbs to the screen reader text too?

#10 follow-up: @SergeyBiryukov
10 years ago

On second thought, 25459.1.diff makes sense, I didn't take brackets into account.

Still, "Edit (Date)" wouldn't sound natural in Russian, if "Date" is translated as a separate word (without a context). Could we perhaps make that "Date: Edit" (same for other links)?

Last edited 5 years ago by SergeyBiryukov (previous) (diff)

#11 in reply to: ↑ 9 @rianrietveld
10 years ago

Replying to SergeyBiryukov:

Replying to toscho:

A longer visible link text could help sighted users too: easier to hit.

It would also help i18n.

25459.1.diff isn't really localizable as is, since it's just separate words rather than a string that could be translated into meaningful text. At least a context should be added for the new words, so that a proper grammatical case could be used.

If a longer visible text would seem redundant, could we add the verbs to the screen reader text too?

We suggested that but @nacin did not approve

#12 in reply to: ↑ 10 @rianrietveld
10 years ago

Replying to SergeyBiryukov:

On second thought, 25459.1.diff makes sense, I didn't took brackets into account.

Still, "Edit (Date)" wouldn't sound natural in Russian, if "Date" is translated as a separate word (without a context). Could we perhaps make that "Date: Edit" (same for other links)?

We discussed this with some developers form Croatia too. This is a compromise to get the link as clear as possible in all languages. It's not perfect, but it's better than nothing.

#14 in reply to: ↑ 13 @lucijanblagonic
10 years ago

Replying to SergeyBiryukov:

How about 25459.2.diff?

This solution is also good. Lets go with this?

#15 follow-up: @SergeyBiryukov
10 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 3.7

No new strings are added.

#16 @rianrietveld
10 years ago

Thank Sergey, I agree!

#17 in reply to: ↑ 15 @SergeyBiryukov
10 years ago

Actually, "Revisions:" and "Date:" are new.

#18 follow-up: @helen
10 years ago

Thought: what if each item was prefixed with an icon, no text (I suppose it would probably be hidden text for screen readers, technically), and then the edit link had the full context, e.g. Edit visibility or Edit date or View revisions.

#19 @SergeyBiryukov
10 years ago

  • Milestone changed from 3.7 to 3.8

Replying to helen:

Thought: what if each item was prefixed with an icon, no text (I suppose it would probably be hidden text for screen readers, technically), and then the edit link had the full context, e.g. Edit visibility or Edit date or View revisions.

Sounds great to me. Let's make sure we have a solution that makes the most sense.

#20 in reply to: ↑ 18 @atimmer
10 years ago

The patches have some issues, for example rtl. Whichever side we put the text on it will be on the wrong side for rtl.

Replying to helen:

Thought: what if each item was prefixed with an icon, no text (I suppose it would probably be hidden text for screen readers, technically), and then the edit link had the full context, e.g. Edit visibility or Edit date or View revisions.

I really like this, we should look at it at a broader scale. To make it useable, accessible and translatable.

#21 @SergeyBiryukov
10 years ago

  • Keywords has-patch added
  • Milestone changed from 3.8 to 3.7

Replying to atimmer:

The patches have some issues, for example rtl. Whichever side we put the text on it will be on the wrong side for rtl.

I've tested 25459.2.diff on an RTL install (he_IL), and it actually looks good (the correct text direction is preserved).

The hesitation here was caused by a thought that "Status: Edit" as two separate strings might still not be flexible enough for some languages. But that's exactly what we have now (as a visible text, just not as a proper link for screen readers).

So, 25459.2.diff is the solution that makes the most sense for now. We're not in string freeze yet, and the two new strings, "Revisions:" and "Date:", are trivial to translate.

Moving back to 3.7 for reconsideration. I'd like to commit this now (if there are no objections from nacin or helen), as it improves accessibility without affecting i18n, and we can work on helen's idea in a new ticket.

#22 @nacin
10 years ago

This should be confirmed as explicitly tested using a number of specific tools, including JAWS and Dragon NaturallySpeaking. jorbin and I have access to all sorts of tools via DC's library, which is also hosting Accessibility Camp DC tomorrow. I can't make it and Jorbin is sick, but I'll try to at least drop by.

We also never tested whether aria could solve this for us.

Not inclined to rush this — let's get it right rather than just guess.

#23 @nacin
10 years ago

  • Keywords commit removed
  • Milestone changed from 3.7 to 3.8

#24 follow-up: @johnbillion
10 years ago

  • Keywords needs-patch i18n-change added; has-patch removed

At WordCamp London Contributor Day the accessibility group done some research on this issue and we believe we have a solution. To recap the two main issues:

Appending the noun as a separate word (for the benefit of screenreaders) is not acceptable for i18n. The following is not localisable in all languages because words and/or word order can change based on context:

<?php _e( 'Edit' ); ?> <?php _e( 'post date' ); ?>

Prepending the noun along with a colon is not acceptable for i18n either. The noun and/or the verb can change based on context, which this does not allow for:

<?php _e( 'Post date:' ); ?> <?php _e( 'Edit' ); ?>

The only solution from an i18n perspective is to provide both phrases (the one-word verb and the complete phrase) as complete strings.

<?php _e( 'Edit' ); ?>
<?php _e( 'Edit post date' ); ?>

This means we need to hide the complete phrase from the browser UI, and also "hide" the one-word phrase from screenreaders and other assistive technology. After some testing the solution appears to be the aria-hidden attribute:

Indicates that the element and all of its descendants are not visible or perceivable to any user as implemented by the author.

Authors MAY, with caution, use aria-hidden to hide visibly rendered content from assistive technologies only if the act of hiding this content is intended to improve the experience for users of assistive technologies by removing redundant or extraneous content.

Our links should end up structured like this:

<a href="#">
    <span aria-hidden="true"><?php _e( 'Edit' ); ?></span>
    <span class="screen-reader-text"><?php _e( 'Edit post date' ); ?></span>
</a>

The first span is shown in browser UIs but ignored by screenreaders. The second span is read by screenreaders but hidden in browser UIs.

Graham Armfield done some testing at the contributor day and confirmed that this technique works with JAWS and NVDA on IE and Firefox (I'm not sure exactly which versions) on Win7, and provides a great benefit to screenreader users.

#25 in reply to: ↑ 24 @grahamarmfield
10 years ago

Replying to johnbillion:

Graham Armfield done some testing at the contributor day and confirmed that this technique works with JAWS and NVDA on IE and Firefox (I'm not sure exactly which versions) on Win7, and provides a great benefit to screenreader users.

I have now also done some testing with Dragon Naturally Speaking 12 - which is voice recognition software used by some people with motor impairments. It was tested with IE10 on a Windows 7 machine.

The test was successful and the links could be followed as the user would expect. This is great news and I do (cautiously) see this as a good solution going forward.

In fact, with the version of Dragon used, both the short and long link texts are available for use. I believe this to be because Dragon currently contains no support for ARIA and so the 'short' link text is also available even though it is enveloped with a span that uses aria-hidden.

There is some debate about whether Dragon will ever support ARIA (see http://blog.paciellogroup.com/2013/11/short-note-aria-dragon-accessibility/) but if we are going to progress this solution it would be sensible if the 'short' link text was always contained within the 'long' link text.

If Dragon Naturally Speaking ever does start recognising and respecting aria-hidden then accessing the link via the visible (to sighted users) 'short' link will no longer be possible. If the 'long' invisible (to sighted users) link text says something completely different then Dragon users will have difficulty accessing the functionality. It is not however a show stopper, as Dragon users also have a couple of ways to emulate mouse clicks. They won't want to be doing that all the time though - it's a drag.

I will shortly be testing an example page on an iPad using Voiceover - the inbuilt screen reader on iOS.

#26 @grahamarmfield
10 years ago

I have now tested an example page using Voiceover on an iPad and Talkback on a Samsung Android phone.

Voiceover

The results are generally good, as Voiceover obviously interprets the aria-hidden attribute and announces only the 'long' link text.

One downside though is that the visual outline that Voiceover puts round the item that has 'focus' is missing. It actually is there, but only around the 'long' link text that is pushed off the screen. This may affect usability for sighted users using Voiceover.

Talkback

The current version of Talkback obviously doesn't support aria-hidden, so Talkback users get both the 'short' and 'long' link texts concatenated. The current item focus is present and visible though.

#27 @matt
10 years ago

  • Priority changed from normal to low
  • Severity changed from normal to minor

#28 follow-up: @johnbillion
10 years ago

Disappointing to see that an accessibility issue is considered low priority and of minor severity.

#29 in reply to: ↑ 28 @nacin
10 years ago

Replying to johnbillion:

Disappointing to see that an accessibility issue is considered low priority and of minor severity.

I'd tend to agree with low/minor here. It has festered since 2.7 and went unreported (not just unfixed) for something like five years, which speaks to its severity not being very major in and of itself. And when compared to the rest of the project's priorities — and even the priorities for making WordPress more accessible — it's relatively low.

That said, Matt was only trying to add some organizational sanity to the 3.8 ticket report. It is still slated for 3.8, and based on recent comments, it seems ready to go in, as I also communicated to John and Graham at WordCamp London's contributor day. So let's get a patch ready.

#30 follow-up: @GaryJ
10 years ago

Is there any difference for any user agents if the order of hidden / not-hidden strings is different? Unlikely, but might be worth a quick check, before agreeing on a consistent order?

#31 follow-up: @neil_pie
10 years ago

During the discussion on this at the contributor day, it was brought up that it might be an idea to add a core function outputting the two accessibility phrases in short and long form. Something like:

function a11y_long_short( $longform, $shortform ) {
    return '<span aria-hidden="true">' . $shortform . '</span><span class="screen-reader-text">' . $longform . '</span>';
}

Some reasons behind this thinking:

  • There are so many places in the admin where this kind of 'shortform verb, longform verb + subject' technique would improve accessibility, a standardised function makes sense
  • It would provide a function available to themes and plugins to provide a 'current best practice method' for achieving this accessible result
  • The functionality we are trying to achieve is described in the spec for aria-label and/or aria-labelledby attributes. Unfortunately, assistive technology is slow to keep up. This lack of support is the reason we need to use two <span> elements. Controlling this markup within a function would enable us to change the output more easily in the future when the ARIA attributes have greater support.

something like the following ( although I'm not 100% sure that this would be the correct markup. Just an example:

function a11y_long_short( $longform, $shortform ) {
    return '<span aria-label="' . $longform . "">' . $shortform . '</span>';
}
Last edited 10 years ago by neil_pie (previous) (diff)

#32 in reply to: ↑ 31 @toscho
10 years ago

Replying to neil_pie:

During the discussion on this at the contributor day, it was brought up that it might be an idea to add a core function outputting the two accessibility phrases in short and long form.

Very good idea. Bonus: the output can be tested automatically in our unit-tests.

#33 in reply to: ↑ 30 @grahamarmfield
10 years ago

Replying to GaryJ:

Is there any difference for any user agents if the order of hidden / not-hidden strings is different? Unlikely, but might be worth a quick check, before agreeing on a consistent order?

I've flipped round the order of the longform and shortform links and there appears to be no discernible difference in browsers, nor with NVDA screen reader.

#34 @grahamarmfield
10 years ago

Suggest also that the function could include an optional argument to allow users to allocate appropriate class name for the portion for screen readers. This would allow more flexibility for the function to be used in custom themes on the front end where other class names may have been used to indicate screen reader only text. The default could be screen-reader-text.

So how about this for a function?

function a11y_long_short( $longform, $shortform, $classname = 'screen-reader-text' ) {
   return '<span aria-hidden="true">' . $shortform . '</span><span class="' . $classname . '"> ' . $longform . '</span>';
}

Note the addition of a leading space at the start of the long form text to help mitigate situations where AT doesn't recognise the aria-hidden attribute, or cases where the CSS is unavailable for some reason so the screen reader only text becomes visible. Without the leading space the last word of the shortform and the first word of the longform would be concatenated.

I'm not sure in which file a function like that might be best placed to be available in front end too. /wp-includes/functions.php maybe?

If some one could let me know I could prepare a patch with the function in and use the function to solve the original issues from the initial trac ticket.

#35 @grahamarmfield
10 years ago

Just for completeness I wanted to note that at WCLDN we did try a solution using aria-label but the results were underwhelming in most AT that we tried.

Last edited 10 years ago by grahamarmfield (previous) (diff)

#36 @ocean90
10 years ago

  • Milestone changed from 3.8 to Future Release

@grahamarmfield
10 years ago

Patch to add two new functions for use when providing short-form and long-form text.

#37 follow-up: @grahamarmfield
10 years ago

There are two functions now, one to directly echo the two strings - a11y_short_long(), and one to simply return the value for use elsewhere - get_a11y_short_long()

I've also placed the short parameter first and changed the name, to reflect the order in that the strings are output.

Functions are as follows:

function get_a11y_short_long( $shortform, $longform, $classname = 'screen-reader-text' ) {
   return '<span aria-hidden="true">' . $shortform . '</span><span class="' . $classname . '"> ' . $longform . '</span>';
}

function a11y_short_long( $shortform, $longform, $classname = 'screen-reader-text' ) {
   echo get_a11y_short_long( $shortform, $longform, $classname);
}

Now working on patch for the meta boxes.

@grahamarmfield
10 years ago

The patch for the meta boxes using the functions provided by the other patch

#38 @grahamarmfield
10 years ago

The new patch applies the necessary changes to the links in the Publish box - addressing the original issue that this trac ticket refers to.

In my development area with both patches applied, the links now voice successfully for screen readers but maintain the single word visible link. The long-form strings can be translated with correct grammar.

Obviously without the functions patch in place the meta-box patch will fail.

#39 @grahamarmfield
10 years ago

  • Keywords has-patch added; needs-patch removed

#40 in reply to: ↑ 37 ; follow-up: @toscho
10 years ago

Replying to grahamarmfield:

There are two functions now, one to directly echo the two strings - a11y_short_long(), and one to simply return the value for use elsewhere - get_a11y_short_long()

Why do we need another wrapper for echo? It clutters the global namespace and doesn’t really add any value.

#41 in reply to: ↑ 40 @grahamarmfield
10 years ago

Replying to toscho:

Why do we need another wrapper for echo? It clutters the global namespace and doesn’t really add any value.

Discussion of this function has indicated that it could be useful in the front end too - ie in themes. Theme developers (such as myself) are familiar with the function()/get_function() construct (eg bloginfo() and get_bloginfo()). In some cases the string output from this new function will be echoed straight to the screen, and in some cases the string is to be passed to a variable.

That is why I did it.

If WordPress as a whole is moving away from that construct then I wasn't aware.

#42 follow-up: @toscho
10 years ago

  • Keywords 2nd-opinion added

Many other echo wrappers do something special with the output (escape or remove parts) or they use echo depending on an argument (like checked()), that’s not the case here. Let’s wait until someone asks for it.

The other problem is the name. It doesn’t say what the function does or even what it is for. Maybe get_aria_label_text() or something similar? I would like to hear get others opinions.

#43 in reply to: ↑ 42 @grahamarmfield
10 years ago

Replying to toscho:

Many other echo wrappers do something special with the output (escape or remove parts) or they use echo depending on an argument (like checked()), that’s not the case here. Let’s wait until someone asks for it.

OK, happy to lose the two function approach. I just want to get the ticket fixed in 3.8 if we can.

The other problem is the name. It doesn’t say what the function does or even what it is for. Maybe get_aria_label_text() or something similar? I would like to hear get others opinions.

Take your point about the function name. Would really like to steer clear of putting aria and label together though since the aria-label idea is not the solution chosen here.

The function takes a short-hand string and a long-hand string and wraps them in spans and then outputs them into a link, form label, button text, or wherever else.

How about get-formatted-short-long-text-for-a11y()?

One other question I had on adding a new function is who starts a codex page for it?

#44 follow-up: @neil_pie
10 years ago

  • Type changed from defect (bug) to enhancement

Taking into account some of the discussion here, I've provided an advancement on the existing patch. This patch:

  • Tries to remove any notion of 'short' vs. 'long' text and keeps it in the context of 'display on screen' vs 'present to screen readers'
  • Adds some escaping to the HTML that we return
  • Removes the $screen_reader_class as a parameter to be passed in and adds it as a parameter that can be filtered
  • Suggests yet another function name, removing references to 'short','long' and 'a11y'
  • Includes PHP Doc in accordance with http://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/

I think it's important to be as broad as possible about what the function is doing. While it's a real solution to an a11y problem, the specifics of the function just provide the appropriate HTML to display one text string on the screen and another to screen readers.

@neil_pie
10 years ago

#45 follow-ups: @GaryJ
10 years ago

neil_pie, I don't think the class name filter serves any benefit here - there's ~146 occurrences of "screen-reader-text" across all files in WP, so it's fairly well established and understood. Since the WP admin CSS requires a consistent class name, I can't see any reason to make it filterable, even if the intention was to push all instances of "screen-reader-text" through a new function (which wouldn't necessarily be great anyway).

The function name seems tricky to nail down - either quite verbose, or unpredictably abbreviated.

How about basing it on something that, as developers, we already know, such as the img alt attribute (which arguably has some parity to what is being covered here). i.e.:

// Easy function name to remember
function alt( $display_text, $screen_reader_text ) {...}

// or

// Reversed arguments, so that you think of "alt text" e.g. "alt" first, then "text" after.
function alt_text( $screen_reader_text, $display_text ) {...}

#46 in reply to: ↑ 45 ; follow-up: @grahamarmfield
10 years ago

Replying to GaryJ:

The function name seems tricky to nail down - either quite verbose, or unpredictably abbreviated.

How about basing it on something that, as developers, we already know, such as the img alt attribute (which arguably has some parity to what is being covered here). i.e.:

// Easy function name to remember
function alt( $display_text, $screen_reader_text ) {...}

// or

// Reversed arguments, so that you think of "alt text" e.g. "alt" first, then "text" after.
function alt_text( $screen_reader_text, $display_text ) {...}

Sorry, I don't feel that is an appropriate name at all. This function has nothing to do with alt text which many people would take to mean the alt attribute. You couldn't successfully use this function with alt text.

The function was originally proposed to solve the problems of less-than-meaningful link text by providing a way to provide two strings. Perhaps then the function should be called something like:

get_meaningful_links($short_text, $long_text, $class_name = 'screen-reader-text') {...}

(Note that I've kept the class-name parameter here as I'm intending to use this function in themes I build in the future.)

#47 in reply to: ↑ 44 @grahamarmfield
10 years ago

Replying to neil_pie:

Taking into account some of the discussion here, I've provided an advancement on the existing patch. This patch:

  • Tries to remove any notion of 'short' vs. 'long' text and keeps it in the context of 'display on screen' vs 'present to screen readers'

Good idea

  • Adds some escaping to the HTML that we return

Good idea

  • Removes the $screen_reader_class as a parameter to be passed in and adds it as a parameter that can be filtered

Isn't filtering a bit overkill? The direct parameter that defaults to 'screen-reader-text' is easier to use.

  • Suggests yet another function name, removing references to 'short','long' and 'a11y'

OK

I think it's important to be as broad as possible about what the function is doing. While it's a real solution to an a11y problem, the specifics of the function just provide the appropriate HTML to display one text string on the screen and another to screen readers.

OK

#48 in reply to: ↑ 46 @GaryJ
10 years ago

Replying to grahamarmfield:

Sorry, I don't feel that is an appropriate name at all. This function has nothing to do with alt text which many people would take to mean the alt attribute. You couldn't successfully use this function with alt text.

The alt attribute provides a representation for screen readers of a visual asset. My alt() function provides a representation for screen readers of an uncontextual text string. There's a clear similarity between the two concepts, and any misunderstanding about it being something that helps provides image alt text would be wiped out when seeing the three lines of code that returns HTML. Core, plugin and theme authors would understand it fine, and a shorter, easily memorable, name makes it far more likely devs will use it, which is what you want.

The function was originally proposed to solve the problems of less-than-meaningful link text by providing a way to provide two strings.

Sure, but neil_pie's solution takes the specific instance of text in links, and makes it a little more general, where it can be any bit of text that could be represented in an expanded way. It could still work for anchor text, for links, but gives flexibility to be re-used elsewhere too.

#49 in reply to: ↑ 45 @neil_pie
10 years ago

Replying to GaryJ:

neil_pie, I don't think the class name filter serves any benefit here - there's ~146 occurrences of "screen-reader-text" across all files in WP, so it's fairly well established and understood. Since the WP admin CSS requires a consistent class name, I can't see any reason to make it filterable, even if the intention was to push all instances of "screen-reader-text" through a new function (which wouldn't necessarily be great anyway).

Replying to grahamarmfield:

Isn't filtering a bit overkill? The direct parameter that defaults to 'screen-reader-text' is easier to use.

If we don't need to provide developers access to edit, remove or add to the class of this element then it's completely unnecessary. If we do need to provide access to this class then filtering on it is absolutely the correct way to go in my opinion. Passing in a parameter to the function call does not provide enough flexibility, particularly if we are hoping developers will use this function beyond WP-admin.

One thought now occurs to me - this classname a temporary fix ( albeit for the foreseeable future ) to the fact that screen readers do not adequately support ARIA label attributes. Should we be able to switch the HTML output in the future then this classname, and passing it to the function or filtering on it at all will become redundant.

I'd suggest we should agree that the solution be to hard code the 'screen-reader-text' class into the output of the function.

Last edited 10 years ago by neil_pie (previous) (diff)

#50 follow-up: @toscho
10 years ago

I’m already sorry for bringing up the name discussion. :)

alt() is way too vague and misleading, and there will be probably collisions with existing plugins or themes. Keep in mind the new function goes into the global namespace.

What we also should keep in mind: this is just a workaround for the current layout in our Publish box. Not even a good one, because these short redundant links are still not very useful on touch screens or for keyboard access (and technically, they act like buttons).

So I think we should not use name that implies that this is a good solution.

Focus on what it does: it creates two elements, one with an attribute aria-hidden="true". It doesn’t hide anything – that depends on CSS or client implementation.

I would prefer something like get_aria_text_combination( $aria_hidden, $aria_accessible, $classname ). Probably with better variable names. :)

#51 in reply to: ↑ 50 ; follow-up: @neil_pie
10 years ago

Replying to toscho:

What we also should keep in mind: this is just a workaround for the current layout in our Publish box. Not even a good one, because these short redundant links are still not very useful on touch screens or for keyboard access (and technically, they act like buttons).

I wouldn't say that it's limited just to the publish box. There are plenty of tickets open regarding 'action' links ( see http://core.trac.wordpress.org/ticket/26167 for an example ) where a solution to this problem could be applied.

It also provides theme and plugin developers with a sanctioned method for solving this problem. I'd like to not lose sight of that, but I'm not sure that it's appropriate to discuss it solely within the context of this ticket

#52 in reply to: ↑ 51 ; follow-up: @toscho
10 years ago

Replying to neil_pie:

I wouldn't say that it's limited just to the publish box. There are plenty of tickets open regarding 'action' links ( see http://core.trac.wordpress.org/ticket/26167 for an example ) where a solution to this problem could be applied.

Yes, these are similar symptoms of the same problem: the existing UI has no room for meaningful, unique links.

It also provides theme and plugin developers with a sanctioned method for solving this problem.

This is already what I was afraid of. :)

If you have any control over the layout, please avoid such a situation and this workaround. Create unique links which your users can hit on a touch screen or search for per keyboard easily.

The markup we’re discussing here doesn’t really solve the problem described the title, just a small part of it. Remember the accesskey disaster: many people concluded from its name this would be something good to have. It wasn’t. That’s why I think we should not suggest in the name that this a proper solution. It isn’t.

#53 in reply to: ↑ 52 @grahamarmfield
10 years ago

Replying to toscho:

If you have any control over the layout, please avoid such a situation and this workaround. Create unique links which your users can hit on a touch screen or search for per keyboard easily.

The markup we’re discussing here doesn’t really solve the problem described the title, just a small part of it. Remember the accesskey disaster: many people concluded from its name this would be something good to have. It wasn’t. That’s why I think we should not suggest in the name that this a proper solution. It isn’t.

I agree with you, the best solution would be to provide unique, meaningful and visible links in situations like this.

I have discussed this with core WP developers and unfortunately the prevailing attitude at the moment seems to be that changing the visible UI to improve accessibility is not on the agenda. See also http://core.trac.wordpress.org/ticket/25461#comment:35 for a comparable situation on visible labels for input fields.

I wish we did have some control over the layout.

In the absence of the ideal solution, I believe this function provides a way to improve accessibility for some. get_aria_text_combination() works for me.

#54 @bramd
10 years ago

  • Cc bram@… added

@GrahamArmfield
10 years ago

New patch which combines Neil Pie's functionality with a more concise function name.

#55 @GrahamArmfield
10 years ago

It would be great if this could be put to bed soon as the functionality is required in other places within the admin area - eg for improving meaningful links within selecting and activating themes in wp-admin/themes.php.

#56 @johnbillion
10 years ago

  • Keywords i18n-change 2nd-opinion removed
  • Milestone changed from Future Release to 3.9
  • Owner set to johnbillion
  • Status changed from new to assigned

I'd like to push this into 3.9 early so we can get some solid testing in and therefore decide whether this is the correct solution (which I think it is).

#57 follow-up: @johnbillion
10 years ago

@GrahamArmfield: In comment 26 you mentioned some concerns with Voiceover and Talkback. Are these still valid?

@neil_pie: Do we really need esc_html() inside of this function? Might there be a case where one of the parameters contains markup?

I went forward and backward on the need for a filter on the CSS class name. I've decided the filter is unnecessary because if a theme utilises the function then it should support the class name that it outputs. There's no situation where a theme author would decide to use the function but not support the class name.

#58 @nacin
10 years ago

  • Component changed from Accessibility to Administration
  • Focuses accessibility added

#59 @nacin
10 years ago

  • Component changed from Administration to Posts, Post Types
  • Focuses administration added

#60 in reply to: ↑ 57 @GrahamArmfield
10 years ago

Replying to johnbillion:

@GrahamArmfield: In comment 26 you mentioned some concerns with Voiceover and Talkback. Are these still valid?

I'd say the issue with Talkback isn't so vital, as v few people who need AT will be using an Android phone/tablet - they'll all be on iPhone or iPad.

Which brings me to Voiceover. My thinking is:

If you are fully blind, then it won't matter. If you have some vision then the focus outline missing could be problematic. But this could be an issue with Voiceover, but Voiceover does still read the correct bit.

#61 follow-up: @johnbillion
10 years ago

You know, I'm tempted to put this function in an admin-only file. I can see it getting used more often than necessary otherwise. It solves the problem we have with the compact UI in this particular situation; I wouldn't want to see it get used more often than necessary.

#62 in reply to: ↑ 61 @toscho
10 years ago

Replying to johnbillion:

You know, I'm tempted to put this function in an admin-only file. I can see it getting used more often than necessary otherwise. It solves the problem we have with the compact UI in this particular situation; I wouldn't want to see it get used more often than necessary.

I agree. We should not encourage such hacks, people should build better UIs instead. :)

@johnbillion
10 years ago

@johnbillion
10 years ago

#63 @johnbillion
10 years ago

25459.5.diff: After some discussion with Nacin we've decided to put the markup inline and forgo the template function altogether in order to discourage its use anywhere else.

#64 @johnbillion
10 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 27196:

Add screen-reader friendly variations of some short text links in the Publish meta box. Fixes #25459.

Note: See TracTickets for help on using tickets.