-
Notifications
You must be signed in to change notification settings - Fork 36
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
DOP 4340/1: Homepage Redesign #1016
Conversation
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.
minor comments below but overall looks great!
one minor thing i noticed is that the hero and text are not aligned very well. we set the height of the hero at different viewports and the align-self: end
properties on the heading and paragraph make the margins vary when changing viewport.
for example:
we can take this in another issue, but perhaps we can make the hero image as a background-image property and have strict margins, not alignments, on the heading and paragraph elements.
I think I have addressed the comments - as for the header, this got approved by design, we didn't want to touch too many things in case the homepage gets changed again but I'm happy to create a separate ticket because I do think having set margins would be nice. |
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.
largely LGTM! left a comment regarding:
1 - the column option to be respected
and 2 - the second set of card groups to have bigger icons
flex-direction: row; | ||
padding-left: ${theme.size.medium}; | ||
img { | ||
width: ${theme.size.xlarge}; |
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.
this LGTM!
should merge the content change also !
Stories/Links:
DOP-4340 + DOP-4341 + DOP-4342 + other small tweaks
New homepage! Now including @anabellabuckvar's tweaks to the introduction header. Built off of this branch of
docs-landing
.Staging Links:
Updated staging link
Figma
Notes:
README updates