-
Notifications
You must be signed in to change notification settings - Fork 9
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: make ML Challenge Page mobile friendly #1141
Conversation
@@ -76,17 +76,16 @@ export function Footer() { | |||
) | |||
|
|||
return ( | |||
<footer className="bg-sds-gray-black min-h-[213px] p-sds-xxl flex flex-col flex-shrink-0"> | |||
<div className="flex items-center flex-wrap flex-col sm:flex-row sm:gap-y-sds-xxl"> | |||
<footer className="bg-sds-gray-black min-h-[213px] pt-[41px] pb-sds-xxl px-sds-xl screen-716:px-sds-xxl flex flex-col flex-shrink-0"> |
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.
can be done later, but if 716
is the breakpoint we're going to settle on for phones it might make sense to add it to our tailwind config
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.
Oh I just realized this is the alias 😆. Maybe we should give them better semantics like "phone", "desktop" or whatever, but that's not really relevant to this PR
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.
we've used this pattern in the past for the napari hub because our breakpoints didn't really have good semantics. I think when SDS officially defines semantic breakpoints for their framework, then we can start using that
I'm also curious from @Janeece @kev-zunshiwang on this, we could defer to SDS or figure out semantic breakpoints for the data portal, but I imagine it would take time to holistically determine the right breakpoints and I know mobile hasn't been a priority up until now 🤔
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.
@codemonkey800 SDS has had breakpoints on its backlog for a long time -- im not sure if that work has been completed (Connor was scoping it, last I heard). We'd want to defer to SDS when they have defined them.
Agreed, mobile has not been generally considered a priority, ML challenge page was a special case because of how the page is promoted.
@@ -76,17 +76,16 @@ export function Footer() { | |||
) | |||
|
|||
return ( | |||
<footer className="bg-sds-gray-black min-h-[213px] p-sds-xxl flex flex-col flex-shrink-0"> | |||
<div className="flex items-center flex-wrap flex-col sm:flex-row sm:gap-y-sds-xxl"> | |||
<footer className="bg-sds-gray-black min-h-[213px] pt-[41px] pb-sds-xxl px-sds-xl screen-716:px-sds-xxl flex flex-col flex-shrink-0"> |
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.
we've used this pattern in the past for the napari hub because our breakpoints didn't really have good semantics. I think when SDS officially defines semantic breakpoints for their framework, then we can start using that
I'm also curious from @Janeece @kev-zunshiwang on this, we could defer to SDS or figure out semantic breakpoints for the data portal, but I imagine it would take time to holistically determine the right breakpoints and I know mobile hasn't been a priority up until now 🤔
<div className="flex items-center mt-[70px] text-sds-body-s flex-col sm:flex-row gap-y-sds-xxl"> | ||
<div className="hidden sm:block">{legalLinks}</div> | ||
<div className="sm:hidden">{cziLinks}</div> | ||
<div className="flex items-center mt-[36px] screen-716:mt-[70px] text-sds-body-s flex-col screen-716:flex-row gap-y-sds-l screen-716:gap-y-sds-xxl"> |
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.
<div className="flex items-center mt-[36px] screen-716:mt-[70px] text-sds-body-s flex-col screen-716:flex-row gap-y-sds-l screen-716:gap-y-sds-xxl"> | |
<div className="flex items-center mt-9 screen-716:mt-[70px] text-sds-body-s flex-col screen-716:flex-row gap-y-sds-l screen-716:gap-y-sds-xxl"> |
Minor nit: In the footer, should the privacy text/link appear above the CZI/CZII logos? @kev-zunshiwang We have hidden the UXR bottom banner from the mobile page, but it's visible on the desktop. I'd like to propose small text updates for clarity (emphasis to indicate changes only):
|
* main: (24 commits) feat: Add more V2 API fields (#1166) feat: Stop querying publications in V2 temporarily (#1160) feat: Add reference tomogram selector for single annotation download (#1136) feat: Add alignment ID row + tooltip (#1153) feat: upgrade sds colors (#1078) feat: upgrade sds (#1077) chore(main): release web 1.29.0 (#1152) feat: Change download all annotations copy (#1151) feat: Start writing run query for new API (#1149) ci: remove glob path pattern for python client tests workflow (#1143) chore(main): release web 1.28.0 (#1137) fix: Fix not enough padding below key photos (#1140) feat: filter carry over behavior (#1128) feat: Prep FE codebase for 2 API URLs (#1126) docs: Add graphics to data organization page (#1135) feat: Descriptions of datasets and depositions. (#1134) chore(main): release web 1.27.0 (#1127) feat: integrate depositions backend (#1093) docs: Implement SDS colors for dark mode (#1131) feat: Implement new single tomogram download flow (#1120) ...
🤖 I have created a release *beep* *boop* --- ## [1.30.0](web-v1.29.0...web-v1.30.0) (2024-09-23) ### ✨ Features * Add alignment ID row + tooltip ([#1153](#1153)) ([1227fbb](1227fbb)) * Add annotations and tomograms tables empty states ([#1155](#1155)) ([bad73bb](bad73bb)) * Add more V2 API fields ([#1166](#1166)) ([0770a16](0770a16)) * Add reference tomogram selector for single annotation download ([#1136](#1136)) ([d1f694a](d1f694a)) * make ML Challenge Page mobile friendly ([#1141](#1141)) ([c25ceeb](c25ceeb)) * Stop querying publications in V2 temporarily ([#1160](#1160)) ([469b9b8](469b9b8)) * Update view tomogram button icons ([#1170](#1170)) ([087d1aa](087d1aa)) * upgrade sds ([#1077](#1077)) ([d56d716](d56d716)) * upgrade sds colors ([#1078](#1078)) ([75ebe8b](75ebe8b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Just saw this -- what does "the privacy text/link appear above the CZI/CZII logos" mean? |
#818
dev link: https://dev-kira-1.cryoet.dev.si.czi.technology/