-
Notifications
You must be signed in to change notification settings - Fork 5
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
Create components for the page sections #549
Create components for the page sections #549
Conversation
…rename the section components
Visit the preview URL for this PR (updated for commit 5aa1f70): https://estuary-marketing--pr549-brenosalv-tech-debt-8dqgn9ak.web.app (expires Fri, 29 Nov 2024 19:52:23 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 76f6b095a0752e5d9c6c890267f9fdc3e392161e |
…yment options page
…ctor template page
…post template page
<span>{xVendor.name.toUpperCase()}</span>{' '} | ||
<span>VS</span>{' '} | ||
<span>{yVendor.name.toUpperCase()}</span> |
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.
What is driving changing this to uppercase 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.
I applied uppercase to the entire h1
text and forgot to remove this. Thanks.
export const Pretitle = styled.p` | ||
font-weight: 700; | ||
font-size: 20px; | ||
margin-bottom: 32px; | ||
margin: 0; | ||
color: #47506d; | ||
text-transform: uppercase; | ||
|
||
@media (min-width: 1280px) { | ||
font-size: 24px; | ||
line-height: 29px; | ||
} | ||
`; |
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.
Since we are getting stuff to be shared... this one might need some tweaks to make look better
I think we need some space between the icon line and the title
Also - I think we might want to add support for like a "standard" and a "small" size for the title text? Just an idea on how to handle this.
That's the reason. There was a margin: 0;
after the margin-bottom: 32px;
.
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.
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 - there are subtle changes to the heros but they all look in sync and I think they all look good.
#542
Changes
It's a refactor to reuse the container div across various page sections.
HeroSectionDetails
component including the title, description and buttons of the hero sections. use the homepage left column of the hero section as reference.