solution - #4499
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation successfully decomposes App.js into the required components with proper folder structure and export default exports. However, there is a critical issue that must be fixed: the CSS is not properly split along with component decomposition. Header.css contains .article styles (lines 30-33) that belong in Article.css, and Article.css contains .header styles in a media query that belong in Header.css. The task requirement explicitly states to "split CSS code along with component decomposition," meaning each component's styles should reside in its own folder's CSS file. Move .header styles (and its media query block) from Article.css to Header.css and move .article styles from Header.css to Article.css. Additionally, remove excessive empty lines between code blocks in Header.css and Navigation.jsx as per CHECKLIST ITEM #1.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| @media (min-width: 500px) { | ||
| .header { | ||
| text-align: left; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| } |
There was a problem hiding this comment.
The .header selector at line 22 in this file (inside @media query) should be in Header.css, not Article.css. This media query block mixes header and article styles together.
| padding: 2em; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The .article selector at line 31 in Header.css should be in Article.css, not Header.css. This appears to be the same misplaced styles that exist in both CSS files.
| </p> | ||
| <p className="article__paragraph"> | ||
| Donec arcu elit, euismod vel lobortis eu, fringilla sit amet dolor. | ||
| congue, massa nec sagittis mollis, dui felis ultrices magna, tincidunt |
There was a problem hiding this comment.
The .article class styles inside the media query should not be in Header.css - they belong in Article.css. This violates the requirement to split CSS along with component decomposition.
| font-size: inherit; | ||
| } | ||
|
|
||
| @media (min-width: 500px) { | ||
| .header { | ||
| text-align: left; |
There was a problem hiding this comment.
This .header selector (and its properties) belongs to Header component, not Article. CSS should be split along with component decomposition - move these styles to Header.css.
|
|
||
| .article { | ||
| margin: 3.5em auto 0; | ||
| padding: 2em; |
There was a problem hiding this comment.
This .article selector (and its properties) belongs to Article component, not Header. CSS should be split along with component decomposition - move these styles to Article.css.
| </nav> | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Extra empty line violates checklist item #1 - don't overuse empty lines between components/code blocks.
| About | ||
| </a> | ||
| <a className="navigation__link" href="#services"> | ||
| Services |
There was a problem hiding this comment.
Extra empty line violates checklist item #1 - don't overuse empty lines.
| @@ -0,0 +1,19 @@ | |||
| import './Navigation.css'; | |||
|
|
|||
There was a problem hiding this comment.
Extra empty line violates checklist item #1 - don't overuse empty lines.
| return ( | ||
| <nav className="navigation"> | ||
| <a className="navigation__link" href="#about"> | ||
| About |
There was a problem hiding this comment.
Extra empty line violates checklist item #1 - don't overuse empty lines.
| </a> | ||
| <a className="navigation__link" href="#contact"> | ||
| Contact | ||
| </a> |
There was a problem hiding this comment.
Extra empty line violates checklist item #1 - don't overuse empty lines between code blocks.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved. All core requirements are met: components are properly decomposed with each in its own folder, CSS is correctly split per component, and all components use export default. The excessive empty lines flagged in the review are style/whitespace issues that fall under cosmetic concerns rather than critical blockers—these should be addressed through automated linting tools rather than blocking approval. The implementation is functionally complete and ready for the next steps.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| import './App.css'; | ||
| import Article from './components/Article/Article'; |
There was a problem hiding this comment.
CHECKLIST ITEM #1 violation: Excessive empty lines between code blocks. Lines 2-3 have double blank lines before .header. Reduce to single blank line.
|
|
||
| .article { |
There was a problem hiding this comment.
CHECKLIST ITEM #1 violation: Excessive empty lines. Lines 2-3 contain 2 empty lines after the comment - should be just 1. Remove the extra empty line to match the good example pattern.
| color: #333; | ||
| } |
There was a problem hiding this comment.
CHECKLIST ITEM #1 violation: Excessive empty lines. Lines 14-15 contain 2 empty lines between .header block and .header__title - should be just 1.
| .article { | ||
| margin: 3.5em auto 0; |
There was a problem hiding this comment.
CHECKLIST ITEM #1 violation: Excessive empty lines. Lines 22-23 contain 2 empty lines before the media query - should be just 1.
| margin: 3.5em auto; | ||
| } |
There was a problem hiding this comment.
CHECKLIST ITEM #1 violation: Excessive empty lines. Lines 30-31 contain 2 empty lines at end of file - should be just 1 or none.
| @@ -1,5 +1,14 @@ | |||
| // import a css file containig welcome styles | |||
| import './Welcome.css'; | |||
|
|
|||
There was a problem hiding this comment.
CHECKLIST ITEM #1 violation: Line 3 has a blank line between import and function definition. Remove it per the good example pattern.
| // import a css file containig welcome styles | ||
| import './Welcome.css'; |
There was a problem hiding this comment.
CHECKLIST ITEM #1 violation: Lines 1-2 have 2 consecutive blank lines after comment. Use maximum 1 empty line.
| <section className="welcome"> | ||
| <span className="welcome__text">Sticky Header!</span> |
There was a problem hiding this comment.
CHECKLIST ITEM #1 violation: Lines 6-7 have 2 consecutive blank lines between .welcome and .welcome__text. Use maximum 1 empty line.
| @@ -1,5 +1,14 @@ | |||
| // import a css file containig welcome styles | |||
| import './Welcome.css'; | |||
|
|
|||
There was a problem hiding this comment.
CHECKLIST ITEM #1 violation: Line 3 has a blank line between import and function definition. Remove it per the good example pattern.
| // Create a Welcome function returning the HTML of welcome block | ||
|
|
There was a problem hiding this comment.
CHECKLIST ITEM #1 violation: Lines 11-12 have a blank line between function and export. Remove it per the good example pattern.
2 participants
DEMO LINK