-
Notifications
You must be signed in to change notification settings - Fork 2
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(census-spotlight): update navbar to new layout #677
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #677 +/- ##
=======================================
Coverage 77.67% 77.67%
=======================================
Files 88 88
Lines 6772 6772
=======================================
Hits 5260 5260
Misses 1512 1512
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Just some formatting related stuff. Looks great otherwise!
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.
feel free to disregard the formatting related comments for this repo.
LGTM!
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 V good! Thanks so much, @seve !
Just some followup questions to make sure we're all on the same page. Thank you!
@@ -50,7 +50,8 @@ export enum EVENTS { | |||
EXPLORER_GENE_INFO_BUTTON_CLICKED = "EXPLORER_GENE_INFO_BUTTON_CLICKED", | |||
WMG_CLICK_NAV = "WMG_CLICK_NAV", | |||
COLLECTIONS_CLICK_NAV = "COLLECTIONS_CLICK_NAV", | |||
CENSUS_CLICK_NAV = "CENSUS_CLICK_NAV", |
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.
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.
Yep! This was my request, thanks @tihuan for checking 🙏
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.
Awesome thanks for confirming!
CENSUS_DOCUMENTATION_CLICK_NAV = "CENSUS_DOCUMENTATION_CLICK_NAV", | ||
CENSUS_DIRECTORY_CLICK_NAV = "CENSUS_DIRECTORY_CLICK_NAV", |
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 assume she knows about the addition of these two events and safe to assume these are in Plausible too?
export const spacesXxl = (props: CommonThemeProps) => getSpaces(props)?.xxl; | ||
export const spacesXl = (props: CommonThemeProps) => getSpaces(props)?.xl; | ||
export const spacesL = (props: CommonThemeProps) => getSpaces(props)?.l; | ||
export const spacesM = (props: CommonThemeProps) => getSpaces(props)?.m; | ||
export const spacesS = (props: CommonThemeProps) => getSpaces(props)?.s; | ||
export const spacesXs = (props: CommonThemeProps) => getSpaces(props)?.xs; | ||
export const spacesXxs = (props: CommonThemeProps) => getSpaces(props)?.xxs; | ||
export const spacesXxxs = (props: CommonThemeProps) => getSpaces(props)?.xxxs; | ||
export const spacesDefault = (props: CommonThemeProps) => | ||
getSpaces(props)?.default; | ||
|
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.
thank you SO much for adding these!! Too bad SDS Hackday didn't add these in SDS library, so we don't have to duplicate the work in both Data Portal and Explorer. Hopefully it will be done in 2024!
Change:
Note: I think something is wrong with my fonts locally for explorer, I'm seeing the same thing when pulling dev