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.

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

Crtl-Up/Down Hotkey doesn't change shabad on First/Last line #504

Closed
sarabveer opened this issue May 10, 2020 · 3 comments · Fixed by #566
Closed

Crtl-Up/Down Hotkey doesn't change shabad on First/Last line #504

sarabveer opened this issue May 10, 2020 · 3 comments · Fixed by #566
Assignees
Labels
Effort 1 Simple task (code/non-code). Impacts Some Affects the average end-user. ○ Type Bug Regressions/production issues that do not function as intended.

Comments

@sarabveer
Copy link

Crtl-Up and Crtl-Down changes the line to the first/last line respectively, but doesn't change the shabad when used again

@bhajneet bhajneet added Impacts Some Affects the average end-user. Status: Confirmed ○ Type Bug Regressions/production issues that do not function as intended. labels May 10, 2020
@saihaj saihaj added the Effort 1 Simple task (code/non-code). label May 10, 2020
@bhajneet bhajneet assigned bhajneet and unassigned saihaj May 29, 2020
@bhajneet
Copy link
Member

Looks like this is taking precedence:

image

image

Which maybe doesn't allow these to fire?

image

Any guidance @Harjot1Singh ?

@bhajneet
Copy link
Member

If I delete 183 and 184 from withNavigationHotkeys it starts to work correctly, so I believe the issue is with the hotkeys precedence being over-ridden by NavigationHotkeys.js

No idea on how to solve though

@Harjot1Singh
Copy link
Member

Good investigative work.

A cheeky but acceptable way to solve this would be to disable those hotkeys inside withNavigationHotkeys when in the navigator. The downside of this would be that you'd lose debounce for home/ctrl-up + end/ctrl-down keys. Which I think is OK in this instance since you can still use up/down and the other hotkeys straight after with debounce (but I think if you hit end up up, you'll end up with a transition to the last line and second last line, and then debounce will kick in, instead of a transition to only the last line)

To go about this, note L189 in withNavigationHotkeys. Can't remember how it's passed in, but just pass in { first: null, last: null } for the keymap in Navigator.js. I think it might be suppliable where withNavigationHotkeys( Navigator ) is called, but do check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Effort 1 Simple task (code/non-code). Impacts Some Affects the average end-user. ○ Type Bug Regressions/production issues that do not function as intended.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants