-
Notifications
You must be signed in to change notification settings - Fork 29
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(app-project): remember current workflow in a same-site session cookie #6216
base: master
Are you sure you want to change the base?
feat(app-project): remember current workflow in a same-site session cookie #6216
Conversation
9f982f1
to
ca89b0a
Compare
8f6c365
to
32b8a27
Compare
const activeWorkflows = self.links['active_workflows'] | ||
let singleActiveWorkflow | ||
if (activeWorkflows.length === 1) { | ||
[singleActiveWorkflow] = self.links['active_workflows'] | ||
} | ||
return singleActiveWorkflow | ||
return singleActiveWorkflow || workflowFromCookie || self.selectedWorkflow |
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.
I'm probably missing something obvious, but self.selectedWorkflow
here is sometimes empty, after setting the workflow_id
cookie.
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.
Fixed by making sure that selectedWorkflow
is added to incoming project snapshots.
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.
Client-side routing in Next.js applies a new project snapshot to the store, from page props, on each route change. The incoming snapshot overwrites any local changes you’ve made to the stored project.
7ebaf77
to
a85f46d
Compare
Here's a short video of me navigating South Coast Threatened Fauna with these changes. Note that when I go to Talk, then Screen.Recording.2024-08-18.at.08.58.54.mov |
348092d
to
8e52500
Compare
Thank you @eatyourgreens for your Issue and linked PR (also tagging @yshish as an fyi). As a heads up, the frontend team has a large backlog of work toward new features and code maintenance scheduled for the rest of the year. Please feel free to keep this Issue open / updated, but PR review will happen a few months from now. Apologies for the slow process. |
eee2dea
to
f465265
Compare
setSelectedWorkflow(workflowId) { | ||
if (typeof document !== 'undefined') { | ||
self.selectedWorkflow = workflowId | ||
document.cookie = `workflow_id=${workflowId}; path=/projects/${self.slug}; domain=zooniverse.org; SameSite=Strict` |
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.
SameSite=Strict
means the cookie is only read while navigating the zooniverse.org
domain. For example, it will work when going to Talk and back but it will be ignored if I enter the site from a link in an email, or a link on an external site. In those latter cases, I’ll be asked to choose a workflow, unless one is specified in the link.
That should protect against a case where a project team mails out a new link to their project, but recipients might have a stale workflow stored on their device.
f465265
to
6f3e1bd
Compare
6f3e1bd
to
af2eedf
Compare
af2eedf
to
4dcb03e
Compare
4dcb03e
to
fe8b475
Compare
fe8b475
to
0a3adec
Compare
This would be super useful for the Material Culture of Wills, which just launched. I'm constantly having to pick a workflow, even though I always work on the same workflow (Checking Transcriptions.) |
0a3adec
to
e8688a0
Compare
- Store your selected workflow in a `workflow_id` session cookie on the `zooniverse.org` domain, scoped to the current project. - Set that cookie when you choose a workflow with the `WorkflowSelectButton` component. - Read the cookie in the Project model, and store it as `project.selectedWorkflow`. Fall back to `project.defaultWorkflow` for backwards compatibility. - Use `project.defaultWorkflow` on the Classify page, if there's no workflow in the URL. - Use `props.workflowID` on the Classify page, if there is a workflow in the URL. - Add the project context to tests.
e8688a0
to
0b1e5aa
Compare
workflow_id
session cookie on thezooniverse.org
domain, scoped to the current project.WorkflowSelectButton
component, or when you level up by following a workflow link in Gravity Spy.project.selectedWorkflow
. Return the stored value fromproject.defaultWorkflow
.project.defaultWorkflow
on the Classify page, if there's no workflow in the URL.props.workflowID
on the Classify page, if there is a workflow in the URL.Please request review from
@zooniverse/frontend
team or an individual member of that team.Package
Linked Issue and/or Talk Post
How to Review
On a project with multiple workflows, clicking a link to choose a workflow should now set a project session cookie that remembers the workflow ID.
/classify
, it should use the cookie to load a workflow, then update the window location bar to show that workflow./classify/workflow/[workflowID]
, it should respect the workflow in the URL.https://local.zooniverse.org:3000/projects/abbsta/south-coast-threatened-fauna-recovery-project?env=production
The big change here is that the Classify link in the project navigation should now reflect your selected workflow (either 21967 or 21968.) That workflow should also be remembered in a session cookie, when you leave the React app and return to https://local.zooniverse.org:3000/projects/abbsta/south-coast-threatened-fauna-recovery-project/classify?env=production
If you shut down the browser, then 'restore previous session', the browser will probably restore the session cookie too. Otherwise, ending your browser session should clear session cookies for all projects, and you'll be asked to choose a workflow when you next classify on a project.
I’ve tested with Gravity Spy and South Coast Threatened Fauna, but any project with more than one workflow should work.
Next.js turns off client-side navigation in development mode, so you’ll need to run the app in production mode to test the project navigation menu.
Checklist
PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.
General
yarn panic && yarn bootstrap
ordocker-compose up --build
and FEM works as expectedGeneral UX
Example Staging Project: i-fancy-cats
New Feature