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

blindfold toggle keybinding + initial wait #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vdegenne
Copy link
Contributor

@vdegenne vdegenne commented Dec 9, 2022

In this pull request, I added 2 things :

  • An initial wait in the init function. For some reasons chess.com is updating the chessboard 2 times on page load, it makes the blindfold state useless. I saw you are using a localstorage to save the toggle value, waiting makes sure the blindfold is loaded back on page refresh.

  • A keyboard shortcut (ctrl+b) for toggling on/off the blindfold as well. I've updated the UI to make sure users know about this new shortcut.

What do you think?

@everyonesdesign
Copy link
Owner

Hello! And sorry for the loooong response time.

An initial wait in the init function. For some reasons chess.com is updating the chessboard 2 times on page load, it makes the blindfold state useless. I saw you are using a localstorage to save the toggle value, waiting makes sure the blindfold is loaded back on page refresh.

Could you elaborate on this? Is something not working or is it an optimisation?
For me the blindfold mode is initialised every time correctly and without delays (I cannot see the position of pieces, tested in Chrome).

A keyboard shortcut (ctrl+b) for toggling on/off the blindfold as well. I've updated the UI to make sure users know about this new shortcut.

Initially I was concerned about keys collision with other shortcuts, but in this case I think that's OK.
Do you think it's better to reassign the shortcut to Cmd+B for macOS users? I think Cmd is a more frequently used modifier key there.

It also seems that some tests are failing in this branch, I'm not sure if we need to rebase it or something got broken. I might check later.

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.

None yet

2 participants