fix homepage testimonials font #1568
Conversation
|
@mbilalmughal Here is a screenshot with the updated font size for the title. How do you recommend we address this issue for a long title that wraps? |
ChristopherChudzicki
left a comment
ChristopherChudzicki
left a comment
There was a problem hiding this comment.
I left a few small comments about the title / subtitle for homepage testimonials section.
A more annoying issue is... I believe the changes to the "AttestantBlock" component are currently affecting testimonials on the unit pages, too.
Unit pages:
Homepage
The two cases are currently using the same AttestantBlock component, differentiated by variant=start | end named, I presume, because the image placement is different).
Suggestion: I think we should just have separate Attestant components for the two usages. The styles are generally different between them (font, spacing, color, arrangement of elements).
| return ( | ||
| <Section> | ||
| <Container id="hamster-noises"> | ||
| <HeaderContainer id="hamster-noises"> |
There was a problem hiding this comment.
Hamsters make weird noises, so we probably don't want those noises on the website. Suggest removing this.
No offense to hamsters.
| <Section> | ||
| <Container id="hamster-noises"> | ||
| <HeaderContainer id="hamster-noises"> | ||
| <Typography component="h2" variant="h2"> |
There was a problem hiding this comment.
Looking at Figma, this should be
| <Typography component="h2" variant="h2"> | |
| <Typography component="h2" typography={{xs: "h3", sm: "h2"}}> |
| <Typography variant="body1"> | ||
| Millions of learners are reaching their goals with MIT's non-degree | ||
| learning resources. Here's what they're saying. | ||
| </Typography> |
There was a problem hiding this comment.
Looking at figma, I guess this should have textAlign: center.
Could be styled(...), or Typography has a prop for that.
There was a problem hiding this comment.
The HeaderContainer styled component handles this with alignItems: "center", do we really need a duplicate centering rule?
There was a problem hiding this comment.
It's not duplicate, they do different things.
align-items: centeraligns the purple box within the blue boxtext-align: centerjustifies the text at center instead of left
Note: I set the width of the purple box to illustrate the effect. Currently the effect is really only visible on mobile
gumaerc
force-pushed
the
cg/fix-testimonials-font
branch
from
2acbb19 to
610180e
Compare
ChristopherChudzicki
left a comment
ChristopherChudzicki
left a comment
There was a problem hiding this comment.
Looks good, but probably still want to text align.
|
Merging for carey, he's on vacation |
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/5360
Description (What does it do?)
This PR adjusts the font and layout of the homepage testimonials section to match the current designs in Figma
Screenshots (if appropriate):
How can this be tested?
mit-learnon this branch