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

feature/share-event-page #149

Merged
merged 5 commits into from
Dec 29, 2021

Conversation

tohuynh
Copy link
Collaborator

@tohuynh tohuynh commented Dec 28, 2021

Link to Relevant Issue

This pull request resolves #95

Description of Changes

Add a share button to generate a shareable URL for an event, with optional query parameters for the current session and user-selected timepoint.

This is mostly a copy of the original functionality with some changes to make it type-safe and add the current session to the shareable URL.

Screenshots

(share button)
image

(modal)
image

@tohuynh tohuynh self-assigned this Dec 28, 2021
Copy link
Member

@evamaxfield evamaxfield left a comment

Choose a reason for hiding this comment

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

This all looks good! I have a single question but otherwise ready for merge.

Comment on lines 50 to 56
return [s, t].map((el) => {
if (el !== null && typeof el === "string") {
const n = parseFloat(el);
return isNaN(n) ? 0 : n;
} else {
return 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

My only question for the whole pull request is "what happens if someone shares url?s=1 will that route to session 1 and default to 0 seconds? AND vice-versa, if someone only shares the seconds url?t=100 will that default to session=0?

I think this is the code block that does that but just want to make sure.

Copy link
Collaborator Author

@tohuynh tohuynh Dec 29, 2021

Choose a reason for hiding this comment

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

shares url?s=1 will that route to session 1 and default to 0 seconds?

Yes, but s=1 will route to the second session (s starts at 0).

shares the seconds url?t=100 will that default to session=0?

Yes

This only happens if the user alters the shareable URL though. The generated shareable URL will contain s and t (if start at checkbox is checked and t>0)

@tohuynh tohuynh merged commit b1b5f7b into CouncilDataProject:main Dec 29, 2021
@tohuynh tohuynh deleted the feature/share-event-page branch December 29, 2021 19:27
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.

Add share at timepoint button to event page
2 participants