-
Notifications
You must be signed in to change notification settings - Fork 27
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
Use quick poll data if it exists in CreateElection #388
Conversation
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.
Left some comments
|
||
const QuickPollTemplate = { | ||
const QuickPollTemplate: Election = { |
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.
Looks like the quick poll reset every time?
I think this is usually the correct behavior, but for the following flow it would be nice to have it remember the previous info
- User partially fills out the quick poll, but realizes they want to specify more things
- User is not logged in, so they click "Login for more settings"
- When they come back they should be able to continue where they left off
I see 2 implementation options
- The QuickPoll component checks if session_state is in the url, and only reads localStorage if that's true (kind of hacky, but I think it would work)
- Clicking "login for more settings" directs the user to the admin page, and then the admin page will redirect to the login flow if the user isn't logged in. This way they'll be redirected to admin page with more settings after login
Option 2 requires fewer clicks from the user, but seeing the admin page might be confusing to them, not sure which is better
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.
QuickPoll doesn't get reset. This input to the useLocalStorage hook is just the default value if it doesn't exist in local storage already.
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.
Ok great! Then there are 2 other questions
- What happens if someone partially fills out the quick poll, and then creates a new election using the nav bar? Will the election creation know to ignore the quick poll info?
- (nit) Should we have an expiration setup so that people don't to see their partial quick poll days/weeks into the future?
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.
- No it won't know to ignore it. Do you think it should?
- That's not a bad idea, but I might save that for another PR
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.
hmm, I feel like it should ignore it, but I don't feel too strongly
Everything else looks good so I'll approve. We can create tasks for those items if you think they're worth addressing in the future
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.
Ship it!
Description
Adds generic types to useLocalStorage
In Create Election page, will use quick poll title if it exists and any candidates that were entered.
Screenshots / Videos (frontend only)
Quick poll page with title and candidates entered
Clicking explore more settings and selecting a template will bring you here
Race view
Related Issues
closes #318