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

Todos project / Vera Sjunnesson #445

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

Conversation

Vera-Sjunnesson
Copy link

Copy link

@JohLeo JohLeo left a comment

Choose a reason for hiding this comment

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

Impressive work as always with the limited time you've had. Overall well-structured and readable code. A very good design added to that, job very well done Vera!!!

Comment on lines +8 to +12
<WaveAnimation />
<HeaderWrapper>
<HeaderText>TODO TODO TODO TODO TODO TODO TODO TODO TODO</HeaderText>
</HeaderWrapper>
</>
Copy link

Choose a reason for hiding this comment

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

Such a nice and creative header, well done!

Comment on lines +95 to +98
<ProjectWrapper>
{Array.isArray(lists) && lists.map((list, listIndex) => (
<>
<ProjectHeader key={list.name} style={{ background:`${colorsArray[a, listIndex]}`}}>
Copy link

Choose a reason for hiding this comment

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

A tip is to use key prop on the outermost component: In the Array.map() method, the key prop is currently being set on the ProjectHeader component. It is recommended to set it on the outermost div or fragment component instead. :)

display: flex;
margin:0;
padding:0;
}
Copy link

Choose a reason for hiding this comment

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

Neat structure and nice work with the global styling!

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