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

fix: add census link in explorer #625

Merged
merged 6 commits into from
Jun 21, 2023
Merged

fix: add census link in explorer #625

merged 6 commits into from
Jun 21, 2023

Conversation

joyceyan
Copy link
Contributor

@joyceyan joyceyan commented Jun 16, 2023

Screen.Recording.2023-06-16.at.2.24.56.PM.mov
  • resolved the nav bar disappearing issue by replacing this block of code in app.tsx
              {(seamlessEnabled || datasetMetadataError === null) && (
                <Header tosURL={tosURL} privacyURL={privacyURL} />
              )}

with

<Header tosURL={tosURL} privacyURL={privacyURL} />
  • those changes were not committed to this PR. but doing this will ensure that we're always rendering the Header component, rather than having rendering be dependent on datasetMetadata returning a valid response
  • since explorer is in maintenance mode, i think this is a reasonable workaround for the few times we do want to update explorer, rather than thinking through how to overhaul the local dev experience to mock this endpoint properly
Screen.Recording.2023-06-16.at.4.20.32.PM.mov

updated to include divider
image

@joyceyan joyceyan requested a review from tihuan June 16, 2023 18:30
@joyceyan joyceyan force-pushed the joyce/explorer-nav branch from 4ca8143 to b1d4860 Compare June 16, 2023 18:36
@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Merging #625 (e11afda) into main (2d35450) will not change coverage.
The diff coverage is n/a.

❗ Current head e11afda differs from pull request most recent head ad42964. Consider uploading reports for the commit ad42964 to get more accurate results

@@           Coverage Diff           @@
##             main     #625   +/-   ##
=======================================
  Coverage   77.67%   77.67%           
=======================================
  Files          88       88           
  Lines        6754     6754           
=======================================
  Hits         5246     5246           
  Misses       1508     1508           
Flag Coverage Δ
frontend 77.67% <ø> (ø)
javascript 77.67% <ø> (ø)
smokeTest ?
unitTest 77.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@joyceyan joyceyan force-pushed the joyce/explorer-nav branch from b1d4860 to 99150a4 Compare June 16, 2023 20:28
@tihuan
Copy link
Contributor

tihuan commented Jun 16, 2023

Thanks for the PR, Joyce!

I think we're just missing the divider 🙆‍♂️

Screenshot 2023-06-16 at 1 31 19 PM

Example on Data Portal that we can probably just copy the same code in Explorer 😆 :

https://github.com/chanzuckerberg/single-cell-data-portal/blob/78783d1685cd14d631caee8da25a58bb780b3343/frontend/src/components/Header/components/Nav/index.tsx#L74

Let me know if I can help in any way! Thank you!

@joyceyan joyceyan force-pushed the joyce/explorer-nav branch from c411b92 to bdb11e3 Compare June 16, 2023 21:01
@tihuan
Copy link
Contributor

tihuan commented Jun 20, 2023

Looks like we need to update the test snapshot, since now that the navbar has the divider and the census link element!
Screenshot 2023-06-20 at 1 08 47 PM

Are you able to run e2e tests locally?

If so, the easiest way to update the snapshot is to pass -- -u to your e2e test command

Otherwise a hacky way to do it is to just update the snapshot file directly yourself. E.g., copy the new snapshot string and replace the one in the snapshot file client/__tests__/e2e/__snapshots__/e2e.test.ts.snap

This one:

exports[`did launch page launched 1`] = `"<div class=\\"css-6q0ddy\\"><span class=\\"css-iz9ysv\\"><a href=\\"/\\"><svg width=\\"23\\" height=\\"23\\" viewBox=\\"0 0 23 23\\" fill=\\"none\\" xmlns=\\"http://www.w3.org/2000/svg\\"><path d=\\"M3.10758 5.97949H0.521711C0.233578 5.97949 0 6.21307 0 6.5012V22.1616C0 22.4497 0.233578 22.6833 0.521711 22.6833H3.10758C3.39571 22.6833 3.62929 22.4497 3.62929 22.1616V6.5012C3.62929 6.21307 3.39571 5.97949 3.10758 5.97949Z\\" fill=\\"white\\"></path><path d=\\"M22.6741 22.1613C22.6741 22.4471 22.4382 22.683 22.1523 22.683H6.50102C6.21521 22.683 5.97931 22.4471 5.97931 22.1613V6.50999C5.97931 6.22419 6.21521 5.98828 6.50102 5.98828H22.1523C22.4382 5.98828 22.6741 6.22419 22.6741 6.50999V22.1613ZM9.6086 18.532C9.6086 18.8178 9.84451 19.0537 10.1303 19.0537H18.5231C18.8089 19.0537 19.0448 18.8178 19.0448 18.532V10.1393C19.0448 9.85348 18.8089 9.61757 18.5231 9.61757H10.1303C9.84451 9.61757 9.6086 9.85348 9.6086 10.1393V18.532Z\\" fill=\\"white\\"></path><path d=\\"M22.1569 0H6.4965C6.20837 0 5.97479 0.233578 5.97479 0.521711V3.10758C5.97479 3.39571 6.20837 3.62929 6.4965 3.62929H22.1569C22.445 3.62929 22.6786 3.39571 22.6786 3.10758V0.521711C22.6786 0.233578 22.445 0 22.1569 0Z\\" fill=\\"#8282FF\\"></path></svg></a><span class=\\"css-1o97835\\"><span class=\\"css-6piv1m\\"><a role=\\"button\\" href=\\"/collections\\" class=\\"bp3-button bp3-minimal\\" tabindex=\\"0\\"><span class=\\"bp3-button-text\\">Collections</span></a></span><span class=\\"css-6piv1m\\"><a role=\\"button\\" href=\\"/datasets\\" class=\\"bp3-button bp3-minimal\\" tabindex=\\"0\\"><span class=\\"bp3-button-text\\">Datasets</span></a></span><span class=\\"css-6piv1m\\"><a role=\\"button\\" href=\\"/gene-expression\\" class=\\"bp3-button bp3-minimal\\" tabindex=\\"0\\"><span class=\\"bp3-button-text\\">Gene Expression</span></a></span></span></span><span class=\\"css-13tdomi\\"><span class=\\"css-6piv1m\\"><span class=\\"bp3-popover2-target\\"><a role=\\"button\\" data-testid=\\"menu\\" class=\\"bp3-button bp3-minimal\\" tabindex=\\"0\\"><span class=\\"bp3-button-text\\">Help &amp; Documentation</span><span icon=\\"chevron-down\\" aria-hidden=\\"true\\" class=\\"bp3-icon bp3-icon-chevron-down\\"><svg data-icon=\\"chevron-down\\" width=\\"16\\" height=\\"16\\" viewBox=\\"0 0 16 16\\"><path d=\\"M12 5c-.28 0-.53.11-.71.29L8 8.59l-3.29-3.3a1.003 1.003 0 00-1.42 1.42l4 4c.18.18.43.29.71.29s.53-.11.71-.29l4-4A1.003 1.003 0 0012 5z\\" fill-rule=\\"evenodd\\"></path></svg></span></a></span></span></span></div>"`;

@tihuan
Copy link
Contributor

tihuan commented Jun 21, 2023

Hurray! Appreciate you confirming, Joyce 🙏 LGTMMM 🚀 Congrats on finishing this Explorer ticket 🎆 !!

@joyceyan joyceyan merged commit 202a90f into main Jun 21, 2023
@joyceyan joyceyan deleted the joyce/explorer-nav branch June 21, 2023 16:15
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.

2 participants