Skip to content
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

merging my code with master #43

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

merging my code with master #43

wants to merge 2 commits into from

Conversation

melissawu2022
Copy link

Added stuff to general advice


const AdvicePage = () => (
<Layout>
<div id="banner">
<h1>FIRST YEAR TIPS</h1>
</div>
<div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this div tag since it doesn't appear to do anything


const GeneralAdvice = () => {
return (
<Grid container direction="row" justify="space-between" alignItems="center" style={{maxWidth:"90%", margin:"0 auto"}}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that clicking "read more" has the side-effect of causing the whole row to expand to fit the new text. Something that might fix this is either giving the Grid-row a specific height that is greater than the maximum height when the text is expanded. Or, giving each individual "AdviceBox" this height. It seems that the individual one works better on mobile though

padding: "0 15px 0 15px"
}}>
<h3>{props.heading}</h3>
<p> {isExpanded ? props.expandedBody : props.shortBody} </p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The font is a bit small and hard-to-read try using a font-size of around 1rem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants