search

edrlab / thorium-reader

A cross platform desktop reading app, based on the Readium Desktop toolkit
https://www.edrlab.org/software/thorium-reader/
BSD 3-Clause "New" or "Revised" License
1.64k stars 145 forks source link

some OPDS "buttons" / GUI controls only activate with enter key, not space bar (are they "a" instead of "button" tags?) #2272

Open danielweck opened 1 month ago

danielweck commented 1 month ago
Screenshot 2024-05-20 at 15 14 56
arthur-lemeur commented 1 month ago

Capture d’écran 2024-05-20 à 16 31 52

Yes they are, but I don't think this is an update from the new UI, I think this was already the case in Thorium 2. Should they be buttons instead ?

danielweck commented 1 month ago

"a" for external hyperlinks (like the LCP hint URL, the "what is OPDS" link etc.) obviously make sense. But note that we have to be very careful with links: https://github.com/edrlab/thorium-reader/issues/2233

"button" elements automatically handle "enter" + "space" key (including preventDefault for scroll and other actions) so technically-speaking they are a good affordance once styled appropriately ... but there are a few places in a GUI where "a" offers more accurate navigations semantics (not just a pedantic consideration, but actual WCAG recommendations). Typically buttons open and close dialog modals, refresh GUI content while remaining active themselves (so that they can be clicked several times in a row) ... so an anti-pattern would be to use buttons for swapping the contents of a view (the button itself disappears and the whole GUI fragment is replaced with something else => that's a navigation kind of UX so a link would be more appropriate (just as in the example filed in this issue).

So, the problem is that in Thorium there is a mix of technical approaches, no strict / purist adherence to a standard. Given the prior art and the fact that consistency at this stage is a top priority (e.g. space bar and enter key tend to work everywhere in Thorium, much like when using a non-web GUI), I personally think that continuing to bend the WCAG rules a little is acceptable (screen reader users will perceive buttons and the action triggered will be clear, as long as the keyboard focus isn't reset to top root level everytime!)

danielweck commented 1 month ago

Perfect example here of link vs. button: https://github.com/edrlab/thorium-reader/pull/2286/files#diff-a7df694e28b199bdb42f482b27cc21e905201b5c6a035233f082002685a8f2aa ( I will add more code comments to highlight potential places to harmonise )

panaC commented 4 days ago

All of the <Link> from react-rooter-dom has concerned from the issue :

tag button : https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/common/components/dialog/publicationInfos/tag/tagButton.tsx#L49

header grid/list view : https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/catalog/Header.tsx#L57

breadcrumb : https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/layout/BreadCrumb.tsx#L51

library header : https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/layout/LibraryHeader.tsx#L158

opds header : https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/layout/LibraryLayout.tsx#L223 https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/layout/LibraryLayout.tsx#L263 https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/layout/LibraryLayout.tsx#L306 https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/layout/LibraryLayout.tsx#L322

opds entry : https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/opds/Entry.tsx#L55

opds feed list : https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/opds/FeedList.tsx#L83

opds page navigation :
https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/opds/FeedList.tsx#L83 https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/opds/PageNavigation.tsx#L123 https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/opds/PageNavigation.tsx#L169 https://github.com/edrlab/thorium-reader/blob/0edc73d1ccc252227215bdcf4f5ab6263c511930/src/renderer/library/components/opds/PageNavigation.tsx#L186

danielweck commented 4 days ago

yes, router Links usually make sense as 'a' HTML hyperlinks for navigation interactions, in which case space-bar keystrokes trigger scrolling (if the viewport is scrollable), and the enter key is automatically event-bound to activate the hyperlink (we only add onClick when the href is missing, or to preventDefault on modifier keys such as shift and alt which cause a download action or switch to a blank Webview destination). WCAG for web applications describes quite well the different UX expectations between buttons and hyperlinks, we just need to make things a bit more consistent in Thorium's UI.