Update next and previous labels to use nextImage and previousImage strings (fixes #1087) #1236
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Thanks, @LlGC-mag! See below for a couple small thoughts...
| </button>` | ||
| ); | ||
|
|
||
| if (this.extension.helper.isRightToLeft()) { | ||
| this.$prevButton.prop("title", this.content.next); | ||
| this.$prevButton.prop("title", this.content.nextImage); |
There was a problem hiding this comment.
It looks like we have this logic to swap out the titles based on right-to-left mode, but it doesn't change the screen reader labels. This seems like a bug/oversight that may be worth fixing while we're making changes anyway. I wonder if something along the lines of this.$prevButton.find(".sr-only").text(this.content.nextImage) would do the job (but that's just an untested guess -- my jQuery is a bit rusty).
| @@ -142,7 +142,9 @@ type OpenSeadragonCenterPanelContent = CenterPanelContent & { | |||
| goHome: string; | |||
| imageUnavailable: string; | |||
| next: string; | |||
| nextImage: string; | |||
There was a problem hiding this comment.
I wonder if we should remove the plain next / previous if they are no longer used. Doesn't hurt anything to keep them, and maybe they're actually still used somewhere, but if they're definitely not needed, it might be nice to clear out some redundancy while we're thinking of it.
Fixes #1087 and includes requested changes in PR #1166
Tested using VoiceOver on mac