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

feat: Export getSearchParam and update login fields based on it #334

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

RobertGemmaJr
Copy link
Member

@RobertGemmaJr RobertGemmaJr commented Nov 21, 2023

The login page is now automatically filled out if studyID and participantID are in the URLSearchParams

  • Renames getQueryVariable as getSearchParam
  • getSearchParam is now exported and used to read studyID and participantID from the URLSearchParams
  • Renames state variables inside <Login />
  • Removes generateWaitSet function as it's unused

closes #199

@RobertGemmaJr RobertGemmaJr self-assigned this Nov 21, 2023
Copy link

github-actions bot commented Nov 21, 2023

Visit the preview URL for this PR (updated for commit c32caf6):

https://ccv-honeycomb--pr334-ref-urlsearchparams-yo2eh8ak.web.app

(expires Wed, 13 Dec 2023 14:57:50 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 4ace1dcea913a952d2a1af84b94a4421bf36e610

Copy link
Contributor

@eldu eldu left a comment

Choose a reason for hiding this comment

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

Some questions ~

src/App/App.jsx Outdated
Comment on lines 95 to 96
if (maybeStudyID != null) setStudyID(maybeStudyID);
if (maybeParticipantID != null) setParticipantID(maybeParticipantID);
Copy link
Contributor

Choose a reason for hiding this comment

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

Strict inequality?

Suggested change
if (maybeStudyID != null) setStudyID(maybeStudyID);
if (maybeParticipantID != null) setParticipantID(maybeParticipantID);
if (maybeStudyID !== null) setStudyID(maybeStudyID);
if (maybeParticipantID !== null) setParticipantID(maybeParticipantID);

Comment on lines +15 to +24
// Update local participantID if it changes upstream
useEffect(() => {
setParticipantID(initialParticipantID);
}, [initialParticipantID]);

// Update local studyID if it changes upstream
useEffect(() => {
setStudyID(initialStudyID);
}, [initialStudyID]);

Copy link
Contributor

Choose a reason for hiding this comment

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

Why would it change upstream?

Copy link
Member Author

Choose a reason for hiding this comment

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

I query parameters weren't auto-filling without these useEffect. I think it's because the state in App.jsx wasn't being set on the first render. I don't imagine folks will change the search parameters but if they do it will change the login fields which I think is nice

@RobertGemmaJr RobertGemmaJr merged commit e43ee6a into main Dec 6, 2023
8 checks passed
@RobertGemmaJr RobertGemmaJr deleted the ref-urlSearchParams branch December 6, 2023 15:06
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.

Use URLSearchParams for getQueryVariable
2 participants