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

feat: make ML Challenge Page mobile friendly #1141

Merged
merged 4 commits into from
Sep 21, 2024
Merged

Conversation

kne42
Copy link
Member

@kne42 kne42 commented Sep 14, 2024

@@ -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">
Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

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 🤔

Copy link
Collaborator

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">
Copy link
Contributor

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">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<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">
image

@kandarpksk
Copy link
Collaborator

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):

Share first impressions of the portal, or sign up for invites to future feedback activities in this short form.

* 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)
  ...
github-actions bot added a commit that referenced this pull request Sep 24, 2024
🤖 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>
@kev-zunshiwang
Copy link
Collaborator

Just saw this -- what does "the privacy text/link appear above the CZI/CZII logos" mean?

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.

6 participants