iftechfoundation / ifdb

The software behind the Interactive Fiction Database (IFDB)
Other
23 stars 18 forks source link

Accessible star controls #350

Open dfabulich opened 4 days ago

dfabulich commented 4 days ago

Fixes https://github.com/iftechfoundation/ifdb/issues/490

Based on https://dev.to/grahamthedev/5-star-rating-system-actually-accessible-no-js-no-wai-aria-3idl

Tested on iOS 18 screen reader. It reads display ratings aloud, e.g. "4 and a half stars out of 5", and it supports navigating to each star as a radio button and checking it.

image image image image image image image image
salty-horse commented 4 days ago

LOL I was just working on it. Here's mine, still has a few bugs (see TODO).

I'll review your approach with the knowledge I gained :)

salty-horse commented 4 days ago

UI issues I noticed:

  1. Since the radio buttons have no "confirmation" step, just changing the focus over them with the keyboard changes the selection (and you made it submit). I don't think this is friendly to keyboard users , who then need to scramble to figure out how to undo their action. From an HTTP angle, I don't think data should be POST-ed to a server just by tabbing/moving over elements. I started my own attempt with radio buttons and switched to regular clickable buttons (Goodreads-style) because of this. I believe radio buttons should only be used when you have a form with a separate "submit" button.
  2. If you rated something as 3 stars, then want to change it to 5, It's weird that 3 stars are red, and 2 are yellow. All the stars should be a single "preview" color. If you want to show the old rating, maybe the current star rating (while you're hovering a new rating) should have an underline or a dashed outline per-star.
  3. After rating something, the "remove it" should show up immediately. (The old code tied this important behavior to a mouse-out and the leaveFunc, which is wrong. That mouse-out is only needed to delay changing the "Rate it:" to "Your Rating:", to avoid shifting the stars element to the right, while you are hovering over it.)
  4. Missing ARIA labels: The fieldset should have an aria-label specifying the current rating (or no rating), and updated along with the server. The radio buttons labels should say "1 star" and not just "1".
  5. This is an issue with the current implementation that I think should be fixed, and probably can't be fixed with the radio buttons' "submit on focus change": If there was an issue sending the data to the server, I think the UI should revert back to the state before the click.

Cleanup:

  1. The review page still uses the accessibility setting. It should be changed.
  2. The settings page should say that the accessbility toggle currently does nothing :)
  3. Delete the old star images and the CSS related to them.

General idea from my own attempt:

  1. Move JS code out of individual pages' PHP soup into ifdbutil.js, and add 'use strict;' at the top. It makes it much easier to catch errors, and for linters to do their job. Since use strict makes JS fail on stuff it didn't before, like undefined variables, it should be tested before deploying to production, but it was very useful during development.
alice-blue commented 3 days ago

2. The settings page should say that the accessbility toggle currently does nothing :)

I wonder if there's anyone who likes the drop-down menu and would want to keep using it. If so, the description could be changed to say "Use drop-down menu for star ratings."

Or we could just hide that section entirely.

salty-horse commented 3 days ago

If the new implementation is good enough, there's no need to keep the drop-down. Can we consult with actual users of this feature, even if it's just to get feedback on the new implementation?

For the setting, it might be good to keep for future use...?

dfabulich commented 2 days ago

I started working on this again but didn't really see a clever way to implement this that works equally well on the viewgame page and on the review page, unless I add a "submit" button to viewgame, which barely fits, and doesn't look great.

@salty-horse do you want to take a crack at this?

salty-horse commented 1 day ago

@dfabulich I'm not sure what is the "this" you mean that causes trouble. Something in my list above? I can see how switching from radio to buttons means a bit more work with the review form.

dfabulich commented 1 day ago

"This" is just, this PR, in general.

I began by saying, "OK, I'll turn the radio buttons into buttons" and then I said, "but what about reviews?"

So then I went in to add a "Submit" button on /viewgame but it didn't really fit (especially when the stars are large enough to tap with a finger). Then I got tired of working on it and wandered off!

salty-horse commented 1 day ago

OK, I'll fix the remaining stuff.

BTW, in Goodreads' "review" form, if you change the star rating, it submits it immediately, separately from the rest of the form. So it might be an OK behavior, but I prefer the current one.