MediaElement: Fix duplicate audio icons

[crhallberg]

Example Manifest: https://digital.library.villanova.edu/Item/vudl:1006929/Manifest.

Upstream PR opened: mediaelement/mediaelement-plugins#266.

---

If you load an audio manifest with multiple sources into the UV, the source selector is also a play button icon.

Investigation

It looks like the button icons have been replaced locally with a modified version of the mediaelementplayer.css. This seems to be a fix for the buttons having no icons without this addition. The icons are missing because of some normalization styles for unstyled buttons on Android. Even sneakier, this normalization also exists as a reset in TailwindCSS, which was used to build the example page. So...

Repairs

1. Remove the normalizations.
2. Use the prebuilt CSS for mediaelementplayer.
3. Refactor the example page styles to remove TailwindCSS.

Enhancements

1. Error reduction in source-chooser.
2. Keyboard accessibility improvements for source-chooser.

Concerns

This PR contains a fork of the source-chooser mediaelement plugin. I am going to contribute this upstream once it's been tested and verified, but this part of the project looks long in the tooth....

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
universalviewer	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 22, 2024 4:52pm

[demiankatz]

Thanks, @crhallberg, this certainly looks like an improvement to me.

If anyone else wants to test, you can use this manifest: https://digital.library.villanova.edu/Item/vudl:384795/Manifest

A couple of notes/comments:

1.) I feel like the keyboard behavior still feels a little weird (for example, when I popped open the source selector, I expected to be able to use the arrow keys but instead had to use tab)... but it's still forward progress to move from "completely broken" to "a little weird." Is this related to the part you hope to contribute upstream?

2.) And speaking of the upstream contribution, is the plan to test here, then contribute upstream, then merge this after the upstream contributions become available? Or do we want to merge with the local override file in the interim? I suppose the answer to that may be dependent on how things play out upstream. My preference would be to avoid having the local override file in the project if we can, but I recognize that it may be our only option if upstream adoption is too slow. Whatever happens, I just want to be sure we don't forget about this and leave the work half-completed indefinitely!

[LanieOkorodudu]

Thanks @crhallberg, The source selector icon looks much better now, and it's definitely easier for users to recognize. When I tested it, I was able to use the ↑ and ↓ arrow keys to select a source. However, I agree with @demiankatz about needing to press the Tab key first before using the arrow keys.
Could it be related to auto-focus? If auto-focus were activated when users navigate to the source chooser, it would automatically focus on the element, allowing immediate interaction with the ↑ and ↓ arrow keys.

One concern is that users who expect to immediately use the ↑ and ↓ arrow keys might not realize they need to press Tab first. Would it be helpful to display a brief pop-up message like "Press Tab, then use ↑ and ↓ arrow keys" when navigating to the source chooser? This would assist sighted users, and as UV's accessibility improves, a screen reader could convey this as well. Just a suggestion!

Overall, this is a fantastic improvement, making keyboard navigation much more user-friendly

[demiankatz]

@LanieOkorodudu, I found that the tab/arrow behavior became much more intuitive after @crhallberg's most recent commit (83318ce). It's possible there are still ways of making it more accessible, but to my non-expert perception, it seems good enough for now. I'd be happy to see what happens if this version is submitted upstream to MediaElement, though of course I'm happy to defer to others with more expertise if further adjustment is still needed.

[LanieOkorodudu]

@LanieOkorodudu, I found that the tab/arrow behavior became much more intuitive after @crhallberg's most recent commit (83318ce). It's possible there are still ways of making it more accessible, but to my non-expert perception, it seems good enough for now. I'd be happy to see what happens if this version is submitted upstream to MediaElement, though of course I'm happy to defer to others with more expertise if further adjustment is still needed.

@demiankatz, The fix looks good to me too! My suggestion was just a thought - the key thing is that we can navigate using the keyboard effectively, and this fix seems to achieve that. I’m happy with it and good to merge!

[crhallberg]

@LanieOkorodudu @demiankatz I'm trying to understand your feedback about the Tab - are you expecting the menu to open when the button is focused rather than hitting Enter or Space to trigger a click?

[crhallberg]

Upstream PR opened: mediaelement/mediaelement-plugins#266.

[LanieOkorodudu]

@crhallberg, Yes, the expectation is that when the settings icon is focused, pressing Enter (or Space) would open the menu and highlight the first option, allowing immediate use of the up and down arrow keys to navigate through options.

Currently, I have to press Tab first before the up and down arrows work to select a source, and Enter/Space isn’t triggering the menu selection as expected.
Would it be possible to update the behaviour so that pressing Enter on the focused settings icon brings focus directly to the source settings, with the first option highlighted? This would allow smooth navigation with the arrow keys from there.

[crhallberg]

@LanieOkorodudu this is the intended control flow. What browser are you using for testing?

[LanieOkorodudu]

@crhallberg

Universal.Viewer.Examples.Mozilla.Firefox.2024-10-28.15-07-37.mp4

I press Tab until the focus highlights the source chooser setting, then press Enter to bring up a menu displaying the available source options. Next, I use the Down and Up Arrow keys, which should allow me to cycle through the different source options within the selector.

However, as shown in the video, pressing the Down and Up Arrow keys changes the volume instead of moving through the source options.

Additionally, when I press Enter today, it correctly triggers the source selection, which didn’t happen last week. I tested this behaviour in both Chrome and Firefox.

Since the recent fix was to change the source chooser icon, this specific issue with the arrow keys may be unrelated and could require a separate fix..

Pressing Tab and then Enter to focus on the source chooser setting works fine now, but this time it's just the Up and Down Arrow keys that are misbehaving.

[LanieOkorodudu]

@crhallberg and @demiankatz, Great work! Tested in both chrome and Firefox. Tab, Enter and Up and down keys works very well and smooth. Thanks again @crhallberg

[demiankatz]

Thanks, @LanieOkorodudu! I think we're essentially done here, but since I'm still hoping we can get the upstream fix merged to mediaelement, I'm not going to merge this yet. If we don't get any traction there by next week's Community Call, we can discuss there whether it's better to merge this fix with the local override, or continue to wait.

[demiankatz]

Unfortunately there's been no motion in the upstream PR related to this one. I think maybe it's time to give up and merge our custom version of the code in the interest of fixing the issue. If we do that, though, I have one minor suggestion first....

[demiankatz]

Maybe this comment should be replaced with a link to the upstream pull request, if we're contemplating merging this as-is due to lack of upstream activity. That way, if in future we try to get rid of this file, we understand why it's here and how to know when it's no longer needed.

[demiankatz]

Thanks, @crhallberg, I'm going to approve this for merging now, but I won't merge it myself until we have another opinion or two, just in case anyone else prefers to wait a little longer.