-
Notifications
You must be signed in to change notification settings - Fork 3
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
107 enhancement secondary partners layout updates #113
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
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.
request change for minor padding in comment . I am sorta partial to the logo being staggered, to me the white space within the card seems odd with it centered.. but If you want to change it I wont stop you either.
<Grid item pl={'6%'} {...partnerGridStyle} order={contentOrder}> | ||
<Typography variant="body1" p={'5% 15% 3% 0'}> | ||
<Grid item {...partnerGridStyle} md={7} order={contentOrder}> | ||
<Typography variant="body1" p={'5% 10% 3% 10%'}> |
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 think padding here can be 0% 10% 3% 10%
, also the pt: '20px'
can go.
const partnerGridStyle = {
xs: 12,
md: 6,
display: 'flex',
justifyContent: 'center',
alignItems: 'center',
flexDirection: 'column',
pt: '20px' <----
};
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 pushed a commit to adjust the padding & alignment. I aligned items top, to be more like the figma file.
The CETI card looks a little off to me but not terrible there was no CETI in the initial figma mockup for comparison.
Note, I moved the 5% padding to the parent grid
element. If I do the same padding on both child elements, they don't come out exactly equal. 🥳 I think this is fixable, but it seemed silly to waste too much time chasing it if padding the parent element works to make them the same size.
@AJSterner did you intend to close this? I don't think this issue has been resolved. Edit: Seems like this was closed by mistake. Reopening. |
stagnant PR and things have since been update since. can always reference and PR again if new changes are made. |
This PR:
Resolves #107
Changes for desktop version of
SecondaryPartners
component only<span>
if there was notestimonialAuthor
prop&&
short-circuit convention for consistencySecondaryPartners
componentScreenshots (if applicable):
Here's what it looks like with these changes
Future Steps/PRs Needed to Finish This Work (optional):
N/A