final#4471
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because one core requirement is still missing and several explicit checklist rules are being violated, which makes this submission incomplete for the task. The navigation is still implemented inline inside Header.jsx and styled in Header.css, instead of being extracted into its own Navigation component and folder (src/components/Navigation/Navigation.jsx + Navigation.css), which is expected by the project-structure rule and the decomposition-by-blocks requirement. You also repeatedly violate [CHECKLIST ITEM #1] by having extra blank lines between imports and component definitions (e.g., in src/App.jsx, src/components/Header/Header.jsx, src/components/Welcome/Welcome.jsx) and between related CSS blocks (e.g., .article and .article__paragraph in Article.css, .welcome and .welcome__text in Welcome.css, navigation-related blocks in Header.css); these need to be tightened so related blocks are adjacent.
To fix this:
- Create a dedicated
Navigationcomponent folder (src/components/Navigation) withNavigation.jsxandNavigation.css, move the<nav className="navigation">…</nav>markup and.navigation__*styles there, and then import/render<Navigation />insideHeader.jsxinstead of the inline<nav>block. - Go through
App.jsx,Header.jsx,Welcome.jsx, and your CSS files and remove unnecessary isolated blank lines between imports and components and between related style blocks to comply with [CHECKLIST ITEM #1]. - After adding the
Navigationfiles, make sure they are tracked by git and committed, and update your PR/demo link if your instructions require a specific URL format.
Functionally your existing components (App, Header, Welcome, Article) are structured well and use export default correctly, so once you extract navigation and clean up the spacing, your solution will align well with the task expectations. Keep going—you are very close, and these changes are mainly about matching the required structure and checklist so your decomposition is fully accepted.
✨ 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
| 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 is an extra empty line between the import block and the App function that matches the BAD EXAMPLE pattern.
| // Create an Article function returning the HTML of article block | ||
| function Article() { | ||
| return ( | ||
| <article className="article"> |
There was a problem hiding this comment.
This isolated empty line between imports and the App component matches the overused blank-line pattern from checklist item #1: "Don't overuse empty lines between components." Consider removing extra blank lines so the imports and component definition are grouped more compactly.
| In elementum lorem eget est euismod ornare. Phasellus sit amet | ||
| pellentesque mauris. Aliquam quis malesuada ex. Nullam eu aliquam nibh. | ||
| Mauris molestie, urna accumsan ornare semper, augue nibh posuere lorem, | ||
| vitae feugiat sem magna eget massa. Vivamus quis tincidunt dolor. Fusce | ||
| efficitur, orci non vestibulum consequat, lectus turpis bibendum odio, | ||
| in efficitur leo felis sed justo. Fusce commodo iaculis orci, quis |
There was a problem hiding this comment.
The multiple isolated empty lines between CSS blocks (e.g., between .article and .article__paragraph) match the "Bad example" pattern from checklist item #1 about not overusing empty lines between components/blocks. Try to minimize these so related styles are grouped more tightly.
| font-size: inherit; | ||
| } | ||
|
|
||
| .navigation__link { |
There was a problem hiding this comment.
Navigation is still implemented directly in Header and its styles are defined here. This violates checklist item #2: "[PROJECT STRUCTURE] - create separate folder per component, where you could put all files(styles, components, and so on)" and the description that you "may also create a folder for the Navigation component and move navigation there." Consider moving .navigation__* styles into a Navigation component’s CSS file.
| <nav className="navigation"> | ||
| <a className="navigation__link" href="#about"> | ||
| About | ||
| </a> | ||
| <a className="navigation__link" href="#services"> | ||
| Services | ||
| </a> | ||
| <a className="navigation__link" href="#contact"> | ||
| Contact | ||
| </a> | ||
| </nav> |
There was a problem hiding this comment.
Navigation is implemented inline here inside Header, rather than decomposed into its own component. This violates checklist item #2: "[PROJECT STRUCTURE] - create separate folder per component, where you could put all files(styles, components, and so on)" and the description suggestion to move navigation into a Navigation component. Consider creating a Navigation component folder and rendering <Navigation /> here instead of the <nav> block.
| height: 100vh; | ||
| background: coral; | ||
| } | ||
|
|
There was a problem hiding this comment.
The extra blank line between the .welcome and .welcome__text blocks contributes to the pattern warned about in checklist item #1: Don't overuse empty lines between components. Try to minimize unnecessary blank lines between related CSS blocks.
| /* Put welcome styles here */ | ||
| .welcome { | ||
| height: 100vh; | ||
| background: coral; |
There was a problem hiding this comment.
Similarly to Header.jsx, this extra blank line between imports and the Welcome component definition matches the "BAD EXAMPLE" for checklist item #1 about not overusing empty lines between components. Removing it will keep the file more compact and aligned with the guideline.
|
|
||
| // Create a Welcome function returning the HTML of welcome block | ||
| function Welcome() { | ||
| return ( |
There was a problem hiding this comment.
The extra empty line between .welcome and .welcome__text styles contributes to the "overuse empty lines" pattern described in checklist item #1. Try to reduce unnecessary blank lines between style blocks so your CSS doesn’t match the BAD EXAMPLE structure.
| lerna-debug.log* | ||
|
|
||
| node_modules | ||
| dist | ||
| dist-ssr | ||
| *.local | ||
|
|
||
| # Editor directories and files | ||
| .vscode/* | ||
| !.vscode/extensions.json | ||
| .idea |
There was a problem hiding this comment.
Navigation is still defined inline inside Header. This goes against the project-structure requirement and the description that you "may also create a folder for the Navigation component and move navigation there." Consider extracting <nav className="navigation">…</nav> into its own Navigation component in src/components/Navigation and importing it here.
| logs | ||
| *.log | ||
| npm-debug.log* | ||
| yarn-debug.log* |
There was a problem hiding this comment.
The extra blank line between .welcome and .welcome__text matches the "Bad example" pattern from checklist item #1: Don't overuse empty lines between components. Try grouping these style blocks without isolated empty lines.
3 participants
https://melr1ze.github.io/react_decompose/