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

Store selection state in URL hash #95

Merged
merged 47 commits into from
Jun 6, 2024
Merged

Store selection state in URL hash #95

merged 47 commits into from
Jun 6, 2024

Conversation

andy-sweet
Copy link
Collaborator

@andy-sweet andy-sweet commented Jun 5, 2024

Closes #49
Closes #62 (by making it no longer relevant)

This stores selection state in the URL hash. In order to make this work, we need some state that reflects the selected points/cells/tracks independent of PointCanvas.curTime because the ordering of the React effects when re-hydrating from the entire state can be different compared to when the user is interacting with the application.

Previously, the canvas stored the last selected point indices, which is transient state that depends on PointCanvas.curTime. Here we stored the all the point IDs that were selected by the user (inspired from #93). Alternatively, we considered storing the track IDs that were selected in #91 , but that doesn't work as well with the rest of the application logic.

This PR also updates the URL hash to store some simpler state that not been added like point brightness. It also refactors that code a little to make it easier to keep new state in sync. Though there is still a lot of duplication that could be improved.

It does not fix #30 , which is related, but can be solved independently.

@andy-sweet andy-sweet changed the title [WIP] Store selection state in URL hash Store selection state in URL hash Jun 5, 2024
Copy link
Collaborator

@aganders3 aganders3 left a comment

Choose a reason for hiding this comment

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

This looks good and works in my manual testing, with no blocking bugs. I think we're on the same page about refactoring, etc. so I am happy to see this merged to move us toward feature-complete.

src/components/App.tsx Outdated Show resolved Hide resolved
src/components/App.tsx Show resolved Hide resolved
src/components/App.tsx Show resolved Hide resolved
src/hooks/usePointCanvas.ts Show resolved Hide resolved
src/lib/PointSelector.ts Show resolved Hide resolved
src/lib/BoxPointSelector.ts Outdated Show resolved Hide resolved
@andy-sweet andy-sweet merged commit 8fca7c5 into main Jun 6, 2024
3 checks passed
@andy-sweet andy-sweet deleted the feat-pointId-state branch June 6, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants