-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add a button that copies shareable URL into the clipboard #44
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this 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 is working well for me. I think it can go in when you're happy with the code, but can you make an issue to capture some thoughts on the design possibilities. I think it would be nice to give the user some feedback both on copy and on load.
Are you planning to add selected points to the state? I think you mentioned it might make the PR too big and could be worked on after.
I am still wrapping my head around some of the ideas here, and how we might want to ultimately structure the components. I still think perhaps some of the state should be lifted up, such that we can separate the controls and the canvas into their own components for example. I need to give this more thought though and I think it's out of scope for this PR.
This is ready for another look whenever you are ready. I didn't include selection in the state because this becomes tricky with timing/ordering. I'd rather get this in and untangle some of the selection after you get the track highlighting PR in. I'm not sure why the Vercel deployment failed - I think can't access those logs... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is neat and will be really valuable. It's working well for me, and I think the structure is a good foundation for building as we add more state through new features.
This provides a simple way to share state between two instances of the tool. It captures most of the current state we care about persisting, with the exception of selection because that is timing/order sensitive and may need some related cleanup before we use it as a shareable state.
The approach defines a dataclass for serializing/deserializing the state from the URL's hash. I don't love it because it requires some boilerplate in the class itself and related book-keeping elsewhere (in
state.tsx
) but I think it's probably good enough for now. Open to better ideas though!I encoded the state using
URLSearchParams
to sanitize the strings for usage in a URL. Alternatively, we could use a base64 encoding of the JSON string which could also simplify the (de)serialization code, though will likely be longer than the current encoding.Closes #18