add task solution#4445
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 meets all core task requirements: App.jsx is properly decomposed into Welcome, Header, Article, and Navigation components with their own folders and CSS files, all components use export default, and the component structure is correctly implemented. The stylelint error in Header.css (missing empty line before at-rule on line 21) and the missing .article__title CSS class are style/cosmetic issues that don't affect functionality. Similarly, the leftover template comments and excessive blank lines mentioned in the review are code cleanliness issues. Since the core decomposition requirements are met and there are no critical blockers that prevent task completion or break functionality, these issues should be addressed as you continue improving your code quality. Run npx stylelint 'src/**/*.scss' --fix to automatically fix the CSS formatting issue.
✨ 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
| left: 0; | ||
| right: 0; | ||
| transition: opacity 0.2s ease-in-out; | ||
| text-align: center; |
There was a problem hiding this comment.
The class .article__title is used in Article.jsx (line 11) but no corresponding style exists in Article.css. Consider adding .article__title styles to match the BEM naming convention used in the rest of the CSS file.
| import React from 'react'; | ||
| import './App.css'; | ||
| // Move each BEM block to a separate component (file) and import them here | ||
| import Welcome from './components/Welcome/Welcome'; | ||
| import Header from './components/Header/Header'; | ||
| import Article from './components/Article/Article'; | ||
|
|
There was a problem hiding this comment.
This violates checklist item #1 - Don't overuse empty lines between components. There are excessive blank lines (lines 2, 4) between the comments and the actual code that should be removed.
| import React from 'react'; | ||
| import './App.css'; | ||
| // Move each BEM block to a separate component (file) and import them here | ||
| import Welcome from './components/Welcome/Welcome'; | ||
| import Header from './components/Header/Header'; | ||
| import Article from './components/Article/Article'; |
There was a problem hiding this comment.
Remove these placeholder comments - they are no longer needed now that the component is implemented.
| /* Put article styles here */ | ||
|
|
||
| .article { | ||
| margin: 5em auto 0; | ||
| padding: 1em; |
There was a problem hiding this comment.
These are leftover instruction comments from the template. The actual implementation is already done below, so these comments should be removed.
| /* Put welcome styles here */ | ||
| .welcome { | ||
| height: 100vh; | ||
| background: coral; | ||
| } |
There was a problem hiding this comment.
These are leftover instruction comments from the template that should be removed since the implementation is already complete below.
| import './Navigation.css'; | ||
|
|
||
| function Navigation() { | ||
| return ( | ||
| <nav className="navigation"> |
There was a problem hiding this comment.
These are leftover instruction comments from the template. The actual implementation is already done below, so these comments should be removed.
| .navigation__link { | ||
| display: inline-block; | ||
| outline: none; | ||
| text-decoration: none; | ||
| opacity: 0.7; |
There was a problem hiding this comment.
These are leftover instruction comments from the template. The actual implementation starts at line 6, so these comments should be removed.
| // Create a Welcome function returning the HTML of welcome block | ||
|
|
||
| // Add a default export statement for Welcome component to use it in the other files | ||
|
|
There was a problem hiding this comment.
This violates checklist item #1 - Don't overuse empty lines between components. Remove this extra blank line after the template comments.
| // Add a default export statement for Welcome component to use it in the other files | ||
|
|
||
| import './Welcome.css'; | ||
|
|
There was a problem hiding this comment.
This violates checklist item #1 - Don't overuse empty lines between components. Remove this extra blank line between the import and function declaration.
| </section> | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
This violates checklist item #1 - Don't overuse empty lines between components. Remove this extra blank line between the function and export statement.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation fully meets all task requirements. The App.jsx is properly decomposed into Welcome, Header, Article, and Navigation components, each in their own dedicated folder with corresponding CSS files. All components correctly use export default, and BEM naming conventions are consistently applied throughout (.article__paragraph, .article__title, .header__title, .navigation__link, etc.). The Navigation component is correctly imported and used within Header. Previous issues mentioned in earlier reviews (missing .article__title class and stylelint errors) have been resolved. Great work on the component decomposition!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
DEMO LINK