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

Added branch navigation shortcuts #273

Merged

Conversation

Cankyre
Copy link
Contributor

@Cankyre Cankyre commented Apr 15, 2024

Added the navigation keybinds from #251

image

NOTE: Default keybind to go to start/end (up/down) is remapped to branch start/end and is replaced with shift+up/down

@Cankyre Cankyre force-pushed the navigation-shortcuts branch from bcb9405 to 4e20392 Compare April 15, 2024 16:11
@Cankyre
Copy link
Contributor Author

Cankyre commented Apr 15, 2024

Forgot lint fix, sorry for the mess

@franciscoBSalgueiro
Copy link
Owner

I tried to use "next branch" but it always gives me an error when I use it on the starting position. Maybe adding some tests in store.test.ts would help

TypeError: Cannot read properties of undefined (reading 'fen')
at GameNotation (http://localhost:1420/src/components/boards/GameNotation.tsx:48:21)

@Cankyre Cankyre force-pushed the navigation-shortcuts branch 2 times, most recently from d625598 to 0b52727 Compare April 19, 2024 07:36
@Cankyre
Copy link
Contributor Author

Cankyre commented Apr 19, 2024

I fucked the pr, sorry, should be fixed though :/, I'd squash everything if I knew how

Fixed starting pos bugs + added tests
@Cankyre Cankyre force-pushed the navigation-shortcuts branch from 0b52727 to b18e29c Compare April 19, 2024 07:42
@Cankyre
Copy link
Contributor Author

Cankyre commented Apr 19, 2024

There, single commit, sorry for the mess again ;>

@franciscoBSalgueiro
Copy link
Owner

franciscoBSalgueiro commented May 1, 2024

I'm not convinced by the branch shortcuts. It doesn't feel as good as the lichess way of going to the next branch.

For example here:

  1. e4 e5 (1... d6 2. d4 (2. d3 )) *

If I go to 1... d6 and then press ctrl-shiftleft it goes forward to 2. d4, which is very unintuitive. Lichess simply goes the next/previous node where there's a branching.

@Cankyre
Copy link
Contributor Author

Cankyre commented May 2, 2024

Indeed that is strange and not an expected behavior. In this case it should go to 1... e5. That seems to be a bug, I will investigate.

@franciscoBSalgueiro
Copy link
Owner

  1. e4 e5 (1... e6 2. d3) *

It crashes if you go to d3 and press ctrl-right.

Also, why not always go to the next branching point, instead of having a different behavior based on the number branches of the parent?

@Cankyre
Copy link
Contributor Author

Cankyre commented May 4, 2024

If you always go to the next branching point, there won't be any way to navigate between the variations, making the keyboard navigation incomplete and require the mouse every now and then. A solution can be to have two different shortcuts or a single one that does both actions. I feel like going to the next branching is way less important than switching between branches

@franciscoBSalgueiro
Copy link
Owner

I agree but having both in the same shortcut is a bit confusing. Maybe having tab for switching between branches when it's in a branching point would be better.

@Cankyre
Copy link
Contributor Author

Cankyre commented May 5, 2024

I added two shortcuts for the branching point navigation (shift+right/left) and two for switching branch (x/c ; haven't found better)

P.S. I deleted the obsolete tests and have not took the time to make new ones yet, I can do that if necessary.

@franciscoBSalgueiro
Copy link
Owner

Looks good! thank you

@franciscoBSalgueiro franciscoBSalgueiro merged commit fd24443 into franciscoBSalgueiro:master May 13, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants