search

Pearson-Higher-Ed / pagination

https://pearson-higher-ed.github.io/pagination/
MIT License
0 stars 0 forks source link

Documentation #2

Closed tsrahm closed 7 years ago

tsrahm commented 7 years ago

This branch contains documentation changes. Feel free to review the entire component. As we get closer to a complete component, we wanted to bring in the PDA team to review and request feedback.

wyseguyonline commented 7 years ago

It is a good practice to run copy-utils manually since any attempt to run any other script will fail until copy-utils runs. Unfortunately, this script cannot be run during the postinstall phase due to issues involving applications that have these components as dependencies.

StommePoes commented 7 years ago

We still have a language mismatch. We do not want English-speaking screen reader users who are on an English language page suddenly start hearing French from hidden text in buttons. If they put in a bug report, the developer wouldn't know what they mean-- the button says "Prev" to everyone but the screen reader user. If the French locale is used when the component is called, the plain text inside the button should be French (and visible to everyone), and nothing should be specially available for screen readers.

The only button that should ever have the span with the class pe-sr-only is the currently active button, the one with the blue background colour.

tsrahm commented 7 years ago

What's best practice regarding abbreviations? We want to minimize the size of the button, therefore we used Prev. However, I felt it was better to use Previous for the screen reader as it is less likely to be misunderstood. If Prev is OK for the screen reader, I'll make that change and eliminate the screen reader span. What about abbreviations for foreign words?

StommePoes commented 7 years ago

Ah, is that the reason for it on previous/next buttons? I can see that being valid for Prev/previous, actually. It doesn't make sense for Next (unless your plan is that that's a general code setup so that language with a common abbreviation for "next" has the setup built in?).

For foreign words, and even for English, I'd lean in general towards being more verbose/clear rather than minimalistic because it's a control... but if you get good info that an abbreviation in another language is wildly popular in sites of that language (such as it's actually pretty common for a previous button to have "prev"), then it's likely ok to use the abbreviation.

So, if the extra span setup for prev/next is for giving the full word compared to an abbreviation... that sounds okay.

tsrahm commented 7 years ago

Yes, that extra span is there for full word vs. abbreviation. I included it for Next also for the sake of consistency and so it's there for foreign abbreviations, if desired. Thanks.

StommePoes commented 7 years ago

Be aware, translation teams will need to know that they need to supply both versions (the visible abbreviation and the hidden full word).

tsrahm commented 7 years ago

Hi Mallory, I made the changes per your suggestions. Thanks for the input. Let me know if you find any other issues.

StommePoes commented 7 years ago

okay, only two things and they're mostly notes:

  1. because buttons are focusable, clickable, interactive elements, Firefox may ignore the aria-hidden and users may hear things like the next and prev button names twice. Just be aware of this, if Jack Brandt or someone reports this, it's a known thing and actually people are still arguing about whether that's a bug or not.

  2. There's a new-ish aria thing that could be tacked onto the items getting the active class, called aria-current. aria-current either has a string representing the name of the thing (in our case, "page") which is current (aria-current="page" on the current item), OR it can have a string representing a Boolean function ("true" or "false" (empty string is also false)). Because support is still coming for this, if you add aria-current="true" to the currently active button, don't take out the sr-only text (which looks correct now, great!).

I'm going to leave #2 as an option for your team because it's still new and support is still getting there, but when we know enough AT and browsers support this, in the future we should be able to simply have an aria-current attribute on the current item and won't need to use hidden sr-only text to tell users which button/link/item is the currently-selected one. If this gets a refactor tag some months down the line for any reason, I'll check the status of aria-current and see if we'll integrate it at that time. \o/

StommePoes commented 7 years ago

Approved.