sul-cidr / pianolatron

Development repository for the CIDR Pianolatron application.
https://pianolatron.stanford.edu
8 stars 4 forks source link

Transport button overhaul, other UX changes #215

Closed broadwell closed 5 months ago

broadwell commented 5 months ago

Most of the changes here are meant to improve the legibility and intelligibility of the buttons in the UI, especially the transport control strip above the roll image. At the request of project stakeholders, button sizes are upscaled, button opacities increased (and not just for the transport buttons), and icons with a ring around a central symbol now consist solely of the symbol. The iconography and behavior of the "record" button is more sophisticated, displaying a looping animation during active recording and also indicating when a recording remains in the buffer.

Other changes (most of which were requested by the project PI) include moving the tempo slider to the transport strip in listen mode, which allows for the removal of the right collapsible panel (if the tempo slider really is the only slider control to be available in listen mode) and updates to the default volume levels and ranges and the number of samples.

netlify[bot] commented 5 months ago

Deploy Preview for pianolatron-develop ready!

Name Link
Latest commit bd16c04a59fad2a6a9cbc9fd634f8ca90197a798
Latest deploy log https://app.netlify.com/sites/pianolatron-develop/deploys/664c0f68a6417000086e2dfa
Deploy Preview https://deploy-preview-215--pianolatron-develop.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

simonwiles commented 5 months ago

These changes all look to be improvements, and don't seem to introduce any issues/regressions across the platforms I've tested on. And, as you mentioned, they're mostly PI-led, so...

Perhaps unsurprisingly since I've not looked closely at the new UI elements before, I do have some thoughts/comments/observations. I'll restrict myself to a couple here:

What do you think? I'm going to merge this PR now, but with your approval I would be happy to raise another PR with some further changes along these lines (there are couple of other very small things I'm minded to tweak a tiny bit).

broadwell commented 5 months ago
  • The highlight styles for the main "transport control strip" buttons are pale on pale backgrounds, and might benefit from something different.

Agreed... do you reckon we should make the background of the buttons a dark color when highlighted, or else change the highlight fill colors of the icon SVGs to something darker/non-white (as is only done for the record button now)?

  • This is especially the case on the pale blue background. The search/browse page and the play interface seem to eschew the theme options, unless I'm missing something; I think this is probably a good idea, and would vote for removing the theme selector for the perform interface too, and just going pale blue all the way.

Yeah the idea here (which admittedly might not be fully baked) was that the listen mode should have a consistent color theme (currently locked in as the blue one), while the expert-oriented perform mode would have a different theme (which as of now is still user-selectable), to help users differentiate visually between the two modes.

  • (My favourite is the grey one, fwiw, but the blue is fine 🙂)

Yeah probably we should just settle on one theme for each of the modes (note however there may eventually be a third 'embed/museum' mode) and perhaps also a separate one for the search and documentation pages? I do kind of like the vintage look of the tan-and-cardinal default, but the greyish one is nice too, as is blue... Maybe tan-and-cardinal for the listen mode, grey for the more technically inclined perform mode, and blue for the static pages? Other suggestions are welcome of course.

  • (The theme selector was originally envisaged as a dev. affordance anyway, tbh, with the idea that we'd settle on a theme and then remove it 🤷)

I briefly entertained the notion that we might want to add a "high-contrast" color theme for accessibility and convert the selector into a "high-contrast" toggle... But I think the preference is for all of the color themes to satisfy the a11y contrast checks.

  • Perhaps using the tooltip action rather than just relying on the HTML title attribute might be worthwhile for the transport control buttons?

Yes I think it's worth giving that a try. I'm still a little unsure about how the transport control buttons will fit into the a11y regime, since it might not make sense for all of them to be mapped to the keyboard (the recording functionality is especially tricky in that regard), so then they'd need to be tab-focusable, in addition to having the proper attributes to be screen reader-friendly. Would the tooltip action play well with all of these other affordances?

What do you think? I'm going to merge this PR now, but with your approval I would be happy to raise another PR with some further changes along these lines (there are couple of other very small things I'm minded to tweak a tiny bit).

Thanks -- further changes and refinements would be very welcome!