chanzuckerberg / single-cell-data-portal

The data portal supporting the submission, exploration, and management of projects and datasets to cellxgene.
MIT License
63 stars 12 forks source link

Populate the global tissue filter with the tissue in the tissue card #5673

Closed prathapsridharan closed 10 months ago

prathapsridharan commented 1 year ago

TODO list:

prathapsridharan commented 1 year ago

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

joyceyan commented 1 year ago

General approach:

Since #5674 needs to be completed, I think including this as URL query params makes the most sense.

tihuan commented 1 year ago

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:

  1. Tissue - cellguide/tissues/:id (we're doing this already)
  2. Tissue specific cell type - cellguide/tissues/:id/cell-types/:cellTypeId (this will be new)
  3. Tissue agnostic cell type - cellguide/:id (we're doing this already)

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!

tihuan commented 12 months ago

CC: @ainfeld

ainfeld commented 12 months ago

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 🙏

tihuan commented 11 months ago

Ah @ainfeld mentioned above that analytics might need to be changed quite a bit to adapt to the URL change, since she's gonna be out for a while, who's helping out with analytics in the meantime?

CC: @niknak33 @dsadgat

Thank you!

tihuan commented 11 months ago

Confirmed that Next.js can handle new URL with frontend/src/pages/cellguide/tissues/[tissueId]/cell-types/[cellTypeId].tsx

e.g., https://localhost:3000/cellguide/tissues/UBERON_0002048/cell-types/CL_0001061

tihuan commented 11 months ago

I estimated 8 points to take into the unknown effort involved in updating analytics

The feature related change + tests should be around 5 points

tihuan commented 11 months ago

I'll start working on this ticket tomorrow!

tihuan commented 11 months ago

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

ainfeld commented 10 months ago

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? 🙏

ainfeld commented 10 months ago

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!!

tihuan commented 10 months ago

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: https://pr-6232-frontend.rdev.single-cell.czi.technology/cellguide/tissues/UBERON_0002369/cell-types/CL_0000540

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 /cellguide/:cellTypeId to capture all cell type page visits, we'll now need both /cellguide/:cellTypeId and cellguide/tissues/:tissueId/cell-types/:cellTypeId to capture all cell type pages' visits

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!

tihuan commented 10 months ago

@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!

ainfeld commented 10 months ago

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.

tihuan commented 10 months ago

In prod now!

https://cellxgene.cziscience.com/cellguide/tissues/UBERON_0000178/cell-types/CL_0000003