-
Notifications
You must be signed in to change notification settings - Fork 12
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
Populate the global tissue filter with the tissue in the tissue card #5673
Comments
When a user goes from a tissue card to a cell card, the cell card page should have the tissue from which the user navigated from automatically pre-selected in the global tissue dropdown |
General approach:
Since #5674 needs to be completed, I think including this as URL query params makes the most sense. |
I think this ticket is not ready for development, since engineers still need to decide on an approach. So we can't point this ticket yet! Yeah I think we can expand #5674 to include a new URL path for tissue specific cell type page like so:
This way the link is sharable and state is encoded in the URL, and it will resolve this ticket as well We can also consider expanding SEO sitemap to include the new paths from #2 as part of the ticket! CC: @dsadgat @niknak33 @joyceyan @prathapsridharan Thanks all! |
CC: @ainfeld |
Thanks @tihuan! Please keep me updated on designs and urls as analytics will need to be changed quite a bit to encompass these changes. Thanks 🙏 |
Confirmed that Next.js can handle new URL with e.g., https://localhost:3000/cellguide/tissues/UBERON_0002048/cell-types/CL_0001061 |
I estimated 8 points to take into the unknown effort involved in updating analytics The feature related change + tests should be around 5 points |
I'll start working on this ticket tomorrow! |
The feature part is ready for code review (CC: @atarashansky 🙏), but keeping it in WIP column, so we're aware that we're still missing the analytics piece |
Thank you so much @tihuan! Is there an rdev link for me to play around with the planned updates and figure out what analytics may need to be updated? 🙏 |
From reviewing your comment on October 18, my hunch is no engineering updates will be required for analytics. I'll just need to update how I'm calculating a bunch of current cell guide measurements. This advanced warning is much appreciated!! |
Hey @ainfeld welcome back! Sorry for the slow reply due to upcoming bereavement 🙏 Yes! Rdev links are automatically posted in the PR. (Something like this!) Here's a tissue specific cell type page, for example: Things to look out for is that since the tissue specific cell type page now has a new URL structure, if you were relying on just The Plausible tracking event that we fire when we change tissue is still happening, so hopefully nothing needs to change there. But happy to make any eng adjustments as needed this week (but I'll be off for 2 weeks starting Monday, so someone else might need to take over CC: @dsadgat ) Just let me know! Thank you! |
@ainfeld caught an important bug that once a user is on a tissue page, a click on a cell type link doesn't take the user to the tissue specific cell type page 🤦🏻♂️ My bad for missing that Will add the feature and a test now! |
Thanks so much @tihuan! I can now confirm there is no engineering updates needed for analytics. The updates needed given this change are all on my end. |
TODO list:
_
to:
The text was updated successfully, but these errors were encountered: