fix: split App into components and fix lint issues by olyanderbilokur · Pull Request #4447 · mate-academy/react_decompose

olyanderbilokur · 2026-06-01T16:50:52Z

mateacademy-ai-mentor

Review Summary

Your component decomposition is well-structured with proper folder organization and export default usage throughout. However, there's a critical issue that must be fixed: Navigation is rendered twice in the component tree, which will cause the navigation to appear twice on the page. Specifically: - In src/components/Header/Header.jsx (line 15), Navigation is imported and rendered inside the Header - In src/App.jsx (line 14), Navigation is also rendered as a separate sibling component. According to the original App.jsx structure, Navigation should be a sibling of Header (not nested inside it), so remove the Navigation import and rendering from Header.jsx. The version in App.jsx should remain as-is. Additionally, there are minor formatting issues (extra blank lines in Navigation.jsx and Welcome.css) that should be cleaned up, though these are lower priority than the structural issue above. Once the duplicate Navigation is removed from Header.jsx, the solution should work correctly. The overall component decomposition approach is sound and follows the task requirements well.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-06-01T16:53:33Z

+  margin: 5em auto 0;
+  padding: 1em;
+  font-size: 140%;
+  max-width: 800px;


The Navigation component is imported and rendered inside Header (lines 6 and within the return statement), but it's also rendered separately in App.jsx (line 14). This causes the navigation to appear twice on the page. Remove the Navigation import and rendering from Header.jsx since it should only exist as a direct child of <main> in App.jsx.

mateacademy-ai-mentor · 2026-06-01T16:53:33Z

+import './Header.css';
+import Navigation from '../Navigation/Navigation';
+
+function Header() {
+  return (
+    <header className="header">
+      <h1 className="header__title">Site Name</h1>
+      <Navigation />


Navigation is imported (line 8) and rendered inside the header (line 15). However, the App.jsx also renders as a separate sibling component, causing duplication. The Navigation should only be rendered once - either inside Header or as a standalone component in App, but not both.

mateacademy-ai-mentor · 2026-06-01T16:53:33Z

+
+.welcome__text {
+  margin: 0;
+  font-size: 400%;
+  text-align: center;
+  line-height: 1;
+  padding-top: calc(50vh - 20pt);


Navigation is imported and rendered inside Header (lines 8, 14), but it's also rendered as a separate component in App.jsx. According to the original App.jsx structure, Navigation should be a sibling of Header, not nested inside it. Remove the Navigation import and rendering from Header.jsx.

mateacademy-ai-mentor

Review Summary

The implementation has a structural issue that needs to be addressed. According to the file comments on Article.jsx, Navigation.css, Welcome.css, and Welcome.jsx, Navigation is currently nested inside Header.jsx but should be a separate sibling component rendered in App.jsx alongside Header.

This is a critical issue because:

.header and .navigation are separate BEM blocks - per BEM methodology, they should be separate components at the same hierarchy level
The component structure in App.jsx currently does not include Navigation as a sibling of Header

Required fix:

Remove the <Navigation /> import and rendering from src/components/Header/Header.jsx
Add <Navigation /> to src/App.jsx as a sibling of <Header />

Additionally, there are minor CSS formatting issues mentioned in the general summary regarding extra blank lines in Welcome.css and Navigation.css - these should be cleaned up (single blank line max between rules).

The component decomposition, folder structure, and export default usage throughout the codebase are well-implemented. Once Navigation is moved to be a sibling component as described, the structural requirements will be fully met.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-06-01T18:19:39Z

 // Create an Article function returning the HTML of article block

 // Add a default export statement for Article component to use it in the other files
+
+import React from 'react';
+import './Article.css';
+
+function Article() {
+  return (
+    <article className="article">
+      <h2 className="article__title">Headline</h2>
+      <p className="article__paragraph">
+        In elementum lorem eget est euismod ornare. Phasellus sit amet


Navigation is rendered inside Header, but it should be a sibling component. Remove <Navigation /> from here and add it to App.jsx as a sibling of <Header />.

mateacademy-ai-mentor · 2026-06-01T18:19:39Z

+  color: black;
+  transition: opacity 0.2s ease-in-out;


Navigation is nested inside Header, but .header and .navigation are separate BEM blocks - they should be separate components. Remove the Navigation import and rendering from Header.jsx.

mateacademy-ai-mentor · 2026-06-01T18:19:39Z

+
+.welcome__text {
+  margin: 0;
+  font-size: 400%;
+  text-align: center;
+  line-height: 1;
+  padding-top: calc(50vh - 20pt);
+  display: block;


Navigation is rendered inside Header, but based on the original App.jsx structure, it should be a separate sibling component. Remove <Navigation /> from Header and add it to App.jsx alongside <Header />.

mateacademy-ai-mentor · 2026-06-01T18:19:39Z

+import './Welcome.css';
+
+function Welcome() {
+  return (
+    <section className="welcome">
+      <span className="welcome__text">Sticky Header!</span>
+    </section>
+  );
+}


Navigation is nested inside Header, but .navigation and .header are separate BEM blocks - they should be separate sibling components. Remove the Navigation import and rendering from Header.jsx.

mateacademy-ai-mentor

Review Summary

Your solution is approved as an educational override to help you progress while learning. You've done an excellent job with the component structure - proper folder organization, correct use of export default, and good BEM naming conventions in your CSS files. However, there are two areas to address for full functionality: 1. Header component needs navigation links - Your tests are failing because the Header component is missing the expected navigation links. Add three <a> tags inside a <nav> element with classes navigation and navigation__link, using href="#about", href="#services", and href="#contact". 2. Template comments - Several files contain leftover instructional comments (lines 1-5 in JSX files and line 1 in CSS files) that should be removed now that the implementation is complete. These are common learning points - template comments are helpful during setup but should be cleaned up once the code is implemented. The navigation structure in the Header component is essential for accessibility and user experience. Keep up the good work with your component decomposition.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-06-01T18:58:19Z

 /* Put article styles here */
+.article {
+  max-width: 800px;
+  margin: 5em auto 0;
+  padding: 1em;


These are template comments that should be removed - the implementation already does all of this (imports CSS, creates the component, and exports it).

mateacademy-ai-mentor · 2026-06-01T18:58:19Z

+.navigation__link {
+  display: inline-block;
+
+  padding: 0 0.5em;
+


These are template comments that should be removed - the implementation already does all of this (imports CSS, creates the component, and exports it).

mateacademy-ai-mentor · 2026-06-01T18:58:19Z

+import React from 'react';
+import './Navigation.css';
+
+function Navigation() {
+  return (


These template comments should be removed - the implementation already does all of this (imports CSS, creates component, exports default).

mateacademy-ai-mentor · 2026-06-01T18:58:19Z

 /* Put header styles here */
+.header {
+  position: sticky;
+  top: 0;
+  right: 0;


These are template comments that should be removed - the implementation already does all of this (imports CSS, creates the component, and exports it).

mateacademy-ai-mentor · 2026-06-01T18:58:19Z

 /* Put welcome styles here */
+.welcome {
+  height: 100vh;
+  background: coral;
+}


These are template comments that should be removed - the implementation already does all of this (imports CSS, creates the component, and exports it).

2pasha

lgtm 🔥

*do not forget to deploy your app

fix: split App into components and fix lint issues

96710aa

mateacademy-ai-mentor suggested changes Jun 1, 2026

View reviewed changes

fix: split App into components and fix lint issues

a58addd

olyanderbilokur requested a review from mateacademy-ai-mentor June 1, 2026 18:15

mateacademy-ai-mentor suggested changes Jun 1, 2026

View reviewed changes

fix: split App into components and fix lint issues

d776c25

olyanderbilokur requested a review from mateacademy-ai-mentor June 1, 2026 18:56

mateacademy-ai-mentor suggested changes Jun 1, 2026

View reviewed changes

fix: place Navigation as sibling of Header

62a7a35

olyanderbilokur requested a review from mateacademy-ai-mentor June 1, 2026 19:14

fix: add navigation to Header component

5ed0f60

2pasha approved these changes Jun 2, 2026

View reviewed changes

Conversation

olyanderbilokur commented Jun 1, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

2pasha left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants