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

Md layouts tw migration & unification #13843

Merged
merged 14 commits into from
Sep 17, 2024
Merged

Md layouts tw migration & unification #13843

merged 14 commits into from
Sep 17, 2024

Conversation

pettinarip
Copy link
Member

@pettinarip pettinarip commented Sep 10, 2024

Description

Effort to consolidate similar looking layouts.

  • Created a new base layout called ContentLayout that uses the ContentHero
  • Updated the following layouts to use ContentLayout: Upgrade, Roadmap, Staking, Translatathon, and UseCases
Layout Production Link Preview Link
Upgrade prod pr
Staking - Pools prod pr
Roadmap - Index prod pr
Roadmap - Security prod pr
Translatathon prod pr
Use Cases prod pr

TODO

  • Run initial round of testing and feedback
  • Complete migration of internal components (used in the layouts) to Tailwind
  • Migrate UseCases layout
  • Add story for ContentLayout
  • Remove build command for testing
  • Add banner in use-cases layout

Copy link

netlify bot commented Sep 10, 2024

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit 3c58b42
🔍 Latest deploy log https://app.netlify.com/sites/ethereumorg/deploys/66e446cb716d1400084d6827
😎 Deploy Preview https://deploy-preview-13843--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 43 (🔴 down 5 from production)
Accessibility: 93 (no change from production)
Best Practices: 83 (🔴 down 9 from production)
SEO: 92 (🔴 down 1 from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added content 🖋️ This involves copy additions or edits tooling 🔧 Changes related to tooling of the project labels Sep 10, 2024
@github-actions github-actions bot added the config ⚙️ Changes to configuration files label Sep 10, 2024
@nloureiro
Copy link
Contributor

For the heroes, I made this comparison on Figma.

All looks very good! Great improvement on the UX of different pages

Only 2 points:

  • the hub hero
  • on the use cases all the use cases should use "uses cases" on top of the title to follow the menu structure.

9898
989898

@nloureiro
Copy link
Contributor

@pettinarip does this ones fall under a different template?

my take is that all should follow the DS heroes, and these ones are still different.
@konopkja, any pushback on updating this one to the same hero template? With the gradient background?

Screen Shot 2024-09-12 10 31 48 AM
Screen Shot 2024-09-12 10 31 36 AM
Screen Shot 2024-09-12 10 31 23 AM

@pettinarip
Copy link
Member Author

pettinarip commented Sep 12, 2024

@pettinarip does this ones fall under a different template?

They do, they are different layouts. You can see that the ones that we changed have the left sidenav and the content on the right.

However, we could create a different PR to update the heros for those pages if we all agree. But I wouldn't include that in this PR.

@pettinarip
Copy link
Member Author

Cool! thanks for the early feedback @nloureiro.

  • the hub hero

Ok, let me see how we can keep this.

  • on the use cases all the use cases should use "uses cases" on top of the title to follow the menu structure.

Hmm ok. Got the idea but this section is used for the breadcrumbs and they are supposed to be links. But we don't have a page under /use-cases to make the connection. Personally, I wouldn't like to add a hack to make a section of the breadcrumbs not clickable, just for this case. Let me think more about it.

@konopkja
Copy link
Contributor

lgtm what pablo did, regarding nuno's question about merging all headers according to the template: maybe just worried how the ETH page would look like with gradient, not a very strong stance here.

Personally i think the design without gradient looks clearer and simpler and less corporate, but that is a different conversation.

@nloureiro
Copy link
Contributor

Cool! thanks for the early feedback @nloureiro.

  • the hub hero

Ok, let me see how we can keep this.

  • on the use cases all the use cases should use "uses cases" on top of the title to follow the menu structure.

Hmm ok. Got the idea but this section is used for the breadcrumbs and they are supposed to be links. But we don't have a page under /use-cases to make the connection. Personally, I wouldn't like to add a hack to make a section of the breadcrumbs not clickable, just for this case. Let me think more about it.

True, they are supposed to be breadcrumbs but:

  1. we don´t follow the navigation. If they are supposed to be breadcrumbs "usecase", it's the right work to be there :)
  2. I wanted to avoid this...
    Screen Shot 2024-09-12 02 08 12 PM

this falls out of scope for this PR, but we need to think how to make the navigation and breadcrumbs match.

@nloureiro
Copy link
Contributor

lgtm what pablo did, regarding nuno's question about merging all headers according to the template: maybe just worried how the ETH page would look like with gradient, not a very strong stance here.

Personally i think the design without gradient looks clearer and simpler and less corporate, but that is a different conversation.

Yes, we need to be careful about that one, need some design work to make it work

Regarding removing the gradient, is this what you have in mind?
Screen Shot 2024-09-12 02 14 23 PM

@konopkja
Copy link
Contributor

lgtm what pablo did, regarding nuno's question about merging all headers according to the template: maybe just worried how the ETH page would look like with gradient, not a very strong stance here.
Personally i think the design without gradient looks clearer and simpler and less corporate, but that is a different conversation.

Yes, we need to be careful about that one, need some design work to make it work

Regarding removing the gradient, is this what you have in mind? Screen Shot 2024-09-12 02 14 23 PM

yes but not for this sidebar template, that one looks terrible without further adjustements

@nloureiro
Copy link
Contributor

lgtm what pablo did, regarding nuno's question about merging all headers according to the template: maybe just worried how the ETH page would look like with gradient, not a very strong stance here.
Personally i think the design without gradient looks clearer and simpler and less corporate, but that is a different conversation.

Yes, we need to be careful about that one, need some design work to make it work
Regarding removing the gradient, is this what you have in mind? Screen Shot 2024-09-12 02 14 23 PM

yes but not for this sidebar template, that one looks terrible without further adjustements

😅 like you said, another conversation...

I was trying to see if we could pull a quick hack, but no

@pettinarip pettinarip marked this pull request as ready for review September 13, 2024 14:06
@wackerow wackerow added the needs design approval 🧑‍🎨 Approval from a designer is needed before merging label Sep 16, 2024
@nloureiro nloureiro removed the needs design approval 🧑‍🎨 Approval from a designer is needed before merging label Sep 17, 2024
Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

Let's go!! 🔥 💪
Thanks for this, nice job @pettinarip!

@wackerow wackerow merged commit 7d096ae into dev Sep 17, 2024
9 of 11 checks passed
@wackerow wackerow deleted the layouts-unification branch September 17, 2024 16:10
This was referenced Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config ⚙️ Changes to configuration files content 🖋️ This involves copy additions or edits tooling 🔧 Changes related to tooling of the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants