Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix tabIndex navigation arrows #2501

Closed
wants to merge 1 commit into from
Closed

fix tabIndex navigation arrows #2501

wants to merge 1 commit into from

Conversation

arthur-lemeur
Copy link
Collaborator

Fixes #2495

@llemeurfr, about "I cannot move pages using the keyboard (Next button)" :
Yes you can do it but it is binded to specific keyboard shortcuts which are by default "Ctrl, SHIFT, ArrowLeft" and "Ctrl, SHIFT, ArrowRight" ("keyboardShortcuts.NavigatePreviousOPDSPage" / "keyboardShortcuts.NavigateNextOPDSPage"

@arthur-lemeur arthur-lemeur requested a review from panaC July 31, 2024 06:59
@llemeurfr
Copy link
Contributor

I just tested on the Blbliomedia feed (https://bm.ebibliomedia.ch/v1/home.opds2), which has no "previous page" information:
Shift + Ctrl + ArrowLeft leads to the NEXT page, like Shift + Ctrl + ArrowRight.
To be decided it we consider it a bug (but in this case should Shift + Ctrl + ArrowLeft be inactive or display an error message?) or a feature.

@panaC panaC mentioned this pull request Sep 19, 2024
Open
@panaC
Copy link
Member

panaC commented Sep 19, 2024

I just tested on the Blbliomedia feed (https://bm.ebibliomedia.ch/v1/home.opds2), which has no "previous page" information: Shift + Ctrl + ArrowLeft leads to the NEXT page, like Shift + Ctrl + ArrowRight. To be decided it we consider it a bug (but in this case should Shift + Ctrl + ArrowLeft be inactive or display an error message?) or a feature.

#2554

panaC
panaC reviewed Sep 19, 2024
View reviewed changes
src/renderer/library/components/opds/PageNavigation.tsx
@@ -97,6 +97,7 @@ class PageNavigation extends React.Component<IProps, undefined> {
<button
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to remove the button tag, the React Router Link component render an anchor html element and not need to be surrouned by a button tag. The button element doesn't even allow to trigger the button action with space keydown/keyup.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay an anchor wrapped inside a button allows to disable the link anchor.

@panaC
Copy link
Member

panaC commented Sep 19, 2024

I'm closing this PR. Arthur works on a new global OPDS issue PR.

@panaC panaC closed this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@panaC panaC panaC left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Navigating feeds using keyboard arrows
3 participants
@arthur-lemeur @llemeurfr @panaC