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.

Sign up for GitHub

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

Jump to bottom

Astro/download page [RFR] #1164

Merged
merged 32 commits into from
Dec 7, 2023
Merged

Astro/download page [RFR] #1164

merged 32 commits into from
Dec 7, 2023

Conversation

djacu
Copy link
Member

@djacu djacu commented Nov 23, 2023 •
edited by thilobillerbeck
Loading

To reviewers:
Please check that the NixOS AWS EC2 selection works correctly and updates the table.

  • Fix vertical centering on the tabs.
  • Panels that aren't the full width are not filling the remaining column.
  • Need to add mobil (small screen) styling.
  • Need to extract AMIs into a content file and map over it.
  • Fix tabs not filling the parent.
  • Tabs don't look exactly the same as the original when the screen is small (when the cross over to medium size).

@djacu djacu changed the base branch from master to main November 23, 2023 17:25
@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://655f8b78ced67213393847e8--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://655fa80a8eef9f2bfc7e812b--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://655fa8eeef3d102cc1c3b4bc--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://655ffff3cfd1d868583ca8d8--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://656005118eef9f63d37e80c3--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://65601a0642fc2171cba5d43e--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://65602956be11a977451b1808--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://6560327ccfd1d807d53ca41d--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://65605a7cbc10d411498702d6--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://6560e81d9094611198bbd878--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://6560f34890946119b3bbd880--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://65610e9cfaa039257846ece4--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://656126f71fd7773c0312aa4d--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://6561411dd640294b2881ab77--nixos-homepage.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://656146df593b114c909a5197--nixos-homepage.netlify.app

@djacu djacu changed the title Astro/download page [WIP] Astro/download page [RFR] Nov 25, 2023
@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed on https://656152f1fbcc7551edc65072--nixos-homepage.netlify.app

@djacu
Copy link
Member Author

djacu commented Nov 25, 2023

@garbas @thilobillerbeck This is ready for review. It works as far as I can tell.

One thing I didn't realize was how the tabs for the different section changed when the screen is less wide. I didn't account for this when designing the components. It can probably be fixed but I haven't figured out how to do it yet.

You can see the comparison below. The old way hides the original tabs and unhides these header tags that are used to collapse the content after. The way I have it now is that the just changes the grid layout.

What do you think? Do we want to match the old way?
If so, and either of you want to give it a try, please feel free.
I've been working on this for 3 days and need a break.

OLD

image

NEW

image

@thilobillerbeck
Copy link
Collaborator

thilobillerbeck commented Nov 29, 2023 •
edited
Loading

nice work, Ill do the review later this day

thilobillerbeck
thilobillerbeck requested changes Nov 30, 2023
View reviewed changes
Copy link
Collaborator

@thilobillerbeck thilobillerbeck left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for your work! There are some small things I marked with a comment but nothing too big.

One thing this implementation clearly leaves open is how we deal with hard coded links to things like NixOS releases, I think it would be better if there would be a place to just swap verison numbers on release and all URLS adapt.

Apart from that I like the mobile UI. It is (against what we planned) not a 1:1 port of the original design but it's okay for me. In the future we maybe could add some kind of dropdown at the top with a heading like Choose platform to save on mobile screen space, but it's okay for now. :)

src/components/pages/download/components/DownloadButtonAndSHA.astro Outdated Show resolved Hide resolved
src/components/pages/download/components/recommended.astro Outdated Show resolved Hide resolved
src/content/aws-ec2/options.yaml Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 7, 2023

🚀 Deployed on https://657162e338124645552f9837--nixos-homepage.netlify.app

djacu added 3 commits December 6, 2023 22:47
@djacu
Forgot to bring some styling over after removing the old download but…
0b7ac20 
…ton components. Also, fixed the classList arg for the Button component.
@djacu
I didn't fix the Button component classList arg last commit. I forgot…
e300ed3 
… how to use them properly. Restored to how they used to be. And fixed how they are used in the Download page.
@djacu
Created a Tag class.
86e3ee3
@djacu djacu requested a review from thilobillerbeck December 7, 2023 07:03
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 7, 2023

🚀 Deployed on https://65716e505289454c9ad1d7ec--nixos-homepage.netlify.app

@djacu
Copy link
Member Author

djacu commented Dec 7, 2023

@thilobillerbeck all changes resolved
See 2a2b082 for modifications to the Button class to allow for optional SHA arguments.
See 86e3ee3 for the new Tag class.

@thilobillerbeck
Copy link
Collaborator

looks good, the Button on the /learn page seems to be broken but this is a fix we can incorporate shortly before publishing the site. Thx for the work.

@thilobillerbeck thilobillerbeck merged commit ce93588 into main Dec 7, 2023
2 checks passed
@thilobillerbeck thilobillerbeck deleted the astro/download-page branch December 7, 2023 14:21
@djacu djacu mentioned this pull request Dec 8, 2023
Merged
thilobillerbeck pushed a commit that referenced this pull request Dec 8, 2023
@djacu
Astro/Fix Button component for size lg-full (#1186)
72a95e2 
Fixes the lg-full size in the Button component as mentioned here:
#1164 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thilobillerbeck thilobillerbeck thilobillerbeck approved these changes

@garbas garbas Awaiting requested review from garbas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@djacu @thilobillerbeck