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

WIP: Add custom event for path changes #34

Closed
wants to merge 1 commit into from

Conversation

johnolek
Copy link
Contributor

This PR adds a custom event that fires any time the path changes.

I'm not sure if this is the kind of thing you want to have in the codebase so I thought I would start this PR as a kind of discussion.

For the thing I'm working on I thought it would be interesting to be able to view a game from both players' perspectives at the same time. For this to be useful would require the two boards to be in sync. Having a simple custom event like this allows me to achieve that with a simple event listener:

    pgnViewer.div.addEventListener('pathChange', (event) => {
      reversedViewer.toPath(event.detail.path)
    })
synced.boards.mov

At first I tried to do this by adding click event listeners to the back/forward arrows, but that doesn't account for keyboard nav or clicking on moves in the list, and they also got cleared out after opening the menu since I think the buttons get re-rendered.

Anyway, if this seems like an ok idea to you, I was thinking it might be best to include some kind of abstraction around it, especially since there are one or two other things I might like to keep track of (like board size).

@ornicar
Copy link
Contributor

ornicar commented Jun 15, 2024

The pgn-viewer is based on chessground, which uses callback functions for events.
I think we should stick to that for consistency. Adding a pathChange callback to the pgn-viewer would be useful.

@johnolek
Copy link
Contributor Author

Good point! not sure why I didn’t think of that

@johnolek johnolek closed this Jun 15, 2024
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