-
Notifications
You must be signed in to change notification settings - Fork 44
Revised home page layout #127
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
Conversation
This tweaks copy and layout on the homepage to provide a better visual hierarchy. It also introduces some changes to the Card component and styles to support the new look in cleaner fashion.
✔️ Deploy Preview for pensive-meitner-faaeee ready! 🔨 Explore the source changes: 7452ed8 🔍 Inspect the deploy log: https://app.netlify.com/sites/pensive-meitner-faaeee/deploys/61d85a12a173ed0007383137 😎 Browse the preview: https://deploy-preview-127--pensive-meitner-faaeee.netlify.app |
I was just browsing this and this looks awesome! Some additional small feedback:
These are all off-the-cuff ideas. You should make the final call on everything here. |
|
||
return ( | ||
<section | ||
<div |
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.
This is more semantically correct right? section
refers more to a "section" in a document rather than this..being a grid, which is more of a container. Am I understanding this properly?
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 had the same thought initially, but in practice I noticed a couple of related things that changed my mind.
- In some cases the Grid belongs logically grouped with some other content, e.g. a header, or supplemental text. In those cases, the semantic "section" is all of that content taken together, not just the grid.
- In some cases, the component using the grid already had a higher level notion of section, which resulted in nested sections. Nested sections aren't prohibited, but semantically it was again incorrect, since the section is meant to map to a section of the page that would be shown in a document outline.
src/pages/index.tsx
Outdated
href="/docs/guides/reference-architecture/overview/overview" | ||
icon="/img/icons/refarch.svg" | ||
> | ||
Bootstrap your infrastructure in about a day by letting |
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.
Nit: This feels more like a marketing blurb and less like verbiage that will help customers find docs for interacting with their Ref Arch.
I could see something like: "Got a Ref Arch deployed? Learn all about it" <-- not this exact text...but the gist should be "if you got the ref arch, this is where you can learn about how to absorb it into your org"
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.
Yeah, that's fair. Updated!
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 had a minor nit - but it's not a blocker. So I'm approving this and can re-stamp if we do end up making changes.
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.
LGTM!
This tweaks copy and layout on the homepage to provide a better
visual hierarchy. It also introduces some changes to the Card
component and styles to support the new look in cleaner fashion.