Skip to content

Testing: initial version #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zerocrates opened this issue Apr 23, 2025 · 22 comments
Open

Testing: initial version #1

zerocrates opened this issue Apr 23, 2025 · 22 comments
Assignees

Comments

@zerocrates
Copy link
Member

You'll have to make sure .xml extension and application/xml and/or text/xml types are allowed.

There are two user-controllable options: the embed height and whether to extract metadata.

When metadata extraction is enabled, items that have OHMS XML files added to them should get changed to the Oral History type (unless another type is already selected), and should have data extracted to the following elements, if data for them is present in the XML:

  • Dublin Core, Title
  • Dublin Core, Description
  • Dublin Core, Identifier (from accession in the XML)
  • Dublin Core, Date
  • Dublin Core, Subject
  • Oral History Item Type Metadata, Interviewer
  • Oral History Item Type Metadata, Interviewee
  • Oral History Item Type Metadata, Duration

If any of those item type metadata elements are missing, it should be handled gracefully, just skipping them and not causing an error.

@sharonmleon
Copy link
Member

Works as expected, with exception of core issue with the carousel image viewer.

@sharonmleon
Copy link
Member

@allanaaa @aviannamiller Here are some sample XML files for testing the OHMS Embed plugin.
OHMS-XMLs.zip

Draft documentation is here: https://github.com/omeka/classic-enduser/blob/OHMSEmbed/docs/Plugins/OhmsEmbed.md

@allanaaa
Copy link

The first sample file worked great - player loaded with no problems, metadata appears as expected, etc.

We will have issues with thumbnails and the lightbox gallery, as you've already mentioned.

I added "application/xml" to my allowed list too, as John said, though you don't have that in the docs. Is it needed, or will "text/xml" be all that's required?

@allanaaa
Copy link

Both of these samples have timestamps but not full transcripts. And I forget what the third column is - translation? Do you have a file with all three so we can see those laid out?

@zerocrates
Copy link
Member Author

On the xmls mimetypes it's hard to say for sure... it depends on what the server detects the file as. In practice I think we've seen text/ mostly but both are valid XML mimetypes.

@aviannamiller
Copy link

Thanks for the sample files. I am having the same success uploading/adding/embedding, although I would also be curious to see how transcripts are treated, as @allanaaa mentioned.

When using a file with text block, I am seeing index points being clipped when page zoom is 150%+ or if the window is shrunk horizontally:
Detail of index point list under embedded video. Some text is clipped by container

@allanaaa
Copy link

allanaaa commented May 2, 2025

Okay, I have one with working video, timestamps, and transcripts:
https://dev.omeka.org/amayer/amayer-c/omeka/items/show/6442

The timestamps will jump the video to the correct point but the transcript does not move. Am I mistaken, or should that be working also?

@zerocrates
Copy link
Member Author

Clicking timestamps on the index side should move the transcript... I'll see if I can see what's up with that.

@zerocrates
Copy link
Member Author

Oh sorry I misremembered what it actually does. Clicking the timestamps only moves the video. Clicking the bookmark moves the transcript.

@allanaaa
Copy link

allanaaa commented May 2, 2025

My mistake - I was absolutely clicking the timestamp and not the bookmark icon.

Okay, file shortcode looks good on a page:

https://dev.omeka.org/amayer/amayer-c/omeka/page-test

My only concern is that with the file shortcode there's no link to the item page.

@aviannamiller
Copy link

The viewer controls should be optimized for screen reader use. Some observations based on further testing:

  • toggle-index is currently only being identified by its ID when it receives focus. A state change is never communicated; nothing is stated when the toggle is activated. It should include aria attributes in order to note whether or not the index is active. WAI's model of a switch pattern incorporates aria-checked to this end and could be applicable here.
  • The same is true of the show-info and fullscreen buttons, the latter of which has a duplicate readout from its matching ID and aria-label. If an aria attribute is needed in this case that is intended to match the element ID, it should instead be implemented using aria-labelledby, which should deal with that duplication.
    • I am also getting a duplicate readout on NVDA for the swap-language button due to it having a matching title and aria-label.
  • The info dialog should have an accessible name to avoid repetitive page name readout, which occurs with JAWS.
  • Activating any of the aforementioned controls or the index actions on Android (tested alongside TalkBack) results in focus returning to the overall viewer container. This is not happening on Mac or Windows.

@zerocrates
Copy link
Member Author

OK, pull again.

Latest change removes redundant aria-labels, adds aria-pressed status to the toggle, adds a name to the info dialog.

Not sure I can do anything about the android focus stuff.

@allanaaa
Copy link

allanaaa commented May 6, 2025

I've dropped a few file shortcodes into this page:

https://dev.omeka.org/amayer/amayer-c/omeka/page-test

Where there is only one column in use by the player, can we widen that column to take up more of the available space? Or is that an inbuilt limitation?

@zerocrates
Copy link
Member Author

@allanaaa the column could be forced wider; my philosophy was that extra width doesn't really make it any easier to use

@aviannamiller
Copy link

aviannamiller commented May 6, 2025

Thank you, the changes look great overall.

  • While aria-pressed allows for the state change in toggle-index to be communicated, I believe the button should be further modified to better identify when index vs. transcription is in view for screen reader users; this would lend more clarity to the toggle's function which currently feels devoid of context removed from the visual interface change. There is also currently a double readout of "toggle" that further loses some of the meaning of the original button text in its duplication.
    • If this feels like an appropriate change, I understand the button label is not supposed to change when the state changes, so what might be the best approach to effectively communicate what the toggle is changing when pressed/unpressed?
  • Can the fullscreen button receive an aria-label of (or akin to) "Enter fullscreen"? After exiting fullscreen mode, focus is returned to the button, and NVDA then gives an automatic readout of "Fullscreen". This seems a bit redundant and confusing for the general flow of entering and escaping fullscreen mode.
  • I am also seeing that the embed frames are missing accessible names, which should be implemented via a title, aria-label, or aria-labelledby attribute (the latter of which might make the most sense in this application?).
  • A bit unrelated, but the main metadata in the frame formats the title as <h1>. This would not be semantically correct in the context of a page, in which the site title, as has been the current approach (may need to be rectified in a separate issue), is tagged as <h1>, page titles are <h2>, content like this would logically follow suit as <h3>, etc.

@zerocrates
Copy link
Member Author

  • The button is "toggle index" but could be called "show index"? Then it'd read as "show index toggle pressed" or similar which maybe is clearer? Something small along those lines is my preference.
  • We have "Enter/exit" fullscreen labels I think... I'll look to see if I removed them or something.... Ah I see, the pair we have is "Fullscreen"/"Exit fullscreen". I can add "enter" to the default label for that button.
  • Titles or labels on the frames, unlike the rest the code for that is actually in this plugin. I'm not sure what we'd want as the title though. labelledby in particular, we don't necessarily/usually have an element elsewhere on the page that describes the frame (possibly true for an item page, less likely when used as a shortcode or in an exhibit). We can pull from metadata of the file, like its title, or have generic text like "Oral History Viewer" or something, or in combination with either of those, we could preferentially use the "alt text" that can be set for a file.
  • The frame is its own page so it's a little tricky to have it assume what header level it should fall at. The "player" component within the frame is intended to be a separate piece that's both shared between the S and Classic module/plugin but also usable on its own or in other contexts (it actually lives in a separate repository omeka/ohms.js that gets merged into this one), adding to the trouble there. The HTML5 algorithm for heading levels that relied on nested sections could have been useful here but it's a dead letter and nothing really supports it. If it's a significant issue we could just not use a header element for the title at all.

@aviannamiller
Copy link

Updating button labels to "Enter fullscreen" and "Show index" sounds good. The latter should certainly help level out what the button is communicating in the toggle action.

Titles or labels on the frames, unlike the rest the code for that is actually in this plugin. I'm not sure what we'd want as the title though. labelledby in particular, we don't necessarily/usually have an element elsewhere on the page that describes the frame (possibly true for an item page, less likely when used as a shortcode or in an exhibit). We can pull from metadata of the file, like its title, or have generic text like "Oral History Viewer" or something, or in combination with either of those, we could preferentially use the "alt text" that can be set for a file.

I would think the accessible name should match the titles provided in the metadata. Generic text could be an issue as frames should not really have the same accessible name if they contain distinct resources, which would hinder accessibility in the case of multiple embeds on one page. To avoid any screen reader redundancy with the displayed title vs. accessible name in this case, is there an aria-labelledby solution that could be implemented to reference the existing title from main-metadata, with this example provided by the ACT Rules Community Group in mind, or otherwise a similar way to go about this?

The frame is its own page so it's a little tricky to have it assume what header level it should fall at. The "player" component within the frame is intended to be a separate piece that's both shared between the S and Classic module/plugin but also usable on its own or in other contexts (it actually lives in a separate repository omeka/ohms.js that gets merged into this one), adding to the trouble there. The HTML5 algorithm for heading levels that relied on nested sections could have been useful here but it's a dead letter and nothing really supports it. If it's a significant issue we could just not use a header element for the title at all.

It is important to ensure the title pulling from the metadata and displaying on the viewer is included in page semantics (Accessibility Developer Guide provides an example of headings implemented in relation to external content in iframes), but I do understand how that becomes a bit complicated here due to the different possible implementations of the player portion. To further understand your point here about the shared/separate piece, is the player component linked to the main metadata, which is the portion that contains that semantic tagging? Or, is the metadata unique to the plugin?

Based on discussions with @kimisgold, headings across both Classic and S themes likely need to be shored up a bit to ensure site titles are always <h1> and page titles are <h2> across the board (if we indeed want to stick with that particular hierarchy), with page subheadings following suit as <h3>, etc. I would think it most logical to treat the relevant metadata (which should usually just be the title, correct?) in the frame as <h3>, akin to the heading level item records receive (or at least are supposed to receive), for example.

@zerocrates
Copy link
Member Author

OK, I have the label changes for those buttons done, not yet merged up to this repo though.

The basic situation with metadata is slightly split in how this plugin works. The viewer itself, the inside of the iframe, can only work with what's in the OHMS XML file. That's where the displayed title which is currently an h1 comes from, and the stuff in the "Info" panel.

The outside part, which includes the iframe tag itself and therefore whatever title/label we want to add to it, that would come from the metadata stored in Omeka. We pull out some of the XML metadata when an OHMS file gets added (including the title), but users can go and edit those fields after we do that.

For the iframe accessible name, I don't think we can really do labelledby here as we don't generally/reliably have another string on the page we can point to. So I think we'd have to be thinking about doing it as a title or aria-label.

@allanaaa
Copy link

Some of these issues will also apply to the S module, right? We didn't do any kind of accessibility work on that.

@zerocrates
Copy link
Member Author

Some of these issues will also apply to the S module, right? We didn't do any kind of accessibility work on that.

The plus side of the viewer part being separate is that all these fixes to the viewer will just come in to the S version with a merge. And it's mostly been viewer fixes.

@aviannamiller
Copy link

@zerocrates, thank you for the explanation on where that h1 is coming from. I am a bit unsure of how to proceed here as there should not be a duplicate h1 on a page, and it is disrupting page semantics.

A heading is, however, important to include in relation to the iframe. As WebTech notes in their discussion of embedded iframes, "not all AT can navigate into iframes," and headings do play an important role in navigation for screen reader users. WebTech recommends a heading be implemented before the embed. As part of this plugin, is there a way to lose the semantic tagging within the iframe and instead implement a heading that appears on the page before the constraints of the iframe, or would that more so fall under the realm of user customization and thus not really work here?

I'm happy with the accessible name being implemented as a title or aria-label if aria-labelledby is not appropriate here, so long as this does not create any redundancy for screen reader users. We can certainly flesh that out in testing.

@zerocrates
Copy link
Member Author

Latest push has:

  • button label changes as mentioned before
  • index titles will wrap more aggressively at small widths
  • title attribute for the iframe
  • heading level changed to h3

I think trying to make a heading that's outside the frame isn't going to really work here for various reasons, customization being one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants