Skip to content

Mini Challenge 1#1

Open
roberto-wl wants to merge 5 commits into
masterfrom
Mini-Challenge-1
Open

Mini Challenge 1#1
roberto-wl wants to merge 5 commits into
masterfrom
Mini-Challenge-1

Conversation

@roberto-wl
Copy link
Copy Markdown
Owner

No description provided.

Comment thread package.json Outdated
"react-scripts": "3.4.3"
"react-scripts": "3.4.3",
"styled-components": "^5.2.1",
"yarn": "^1.22.10"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

yarn should not be installed as a project dependency


const Header = styled.header`
color: #fff;
background-color: #1c5476;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I recommend handling the colors with a theme provider, so changing colors with the light and dark theme becomes easier:

https://styled-components.com/docs/advanced#function-themes

font-size: 1rem;
box-sizing: border-box;
align-items: center;
font-family: 'Roboto', 'Helvetica', 'Arial', sans-serif;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see that you are repeating font-size a lot, you can use global styles to define a font family for the whole application: https://styled-components.com/docs/api#createglobalstyle

justify-content: center;
align-items: center;
margin-top: -3rem;
@media (min-width: 1135px)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can also use media queries with styled-components (:


const Container = styled.div`
width: 345px;
height: 345px;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I suggest you use relative units like rem. This helps you achieve a more responsive application. Ideally, you should only use pixels for font sizes, borders, and small stuff.

https://thecssworkshop.com/lessons/relative-units

Comment thread src/global.css
margin: 0;
padding: 0;
text-rendering: optimizeLegibility;
/* text-rendering: optimizeLegibility;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can also handle globals with styled-components: https://styled-components.com/docs/api#createglobalstyle

Comment thread src/global.css
margin: 0;
padding: 0;
text-rendering: optimizeLegibility;
/* text-rendering: optimizeLegibility;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I suggest you remove unused code

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