Skip to content

solution#4284

Open
Skay9n wants to merge 3 commits intomate-academy:masterfrom
Skay9n:develop
Open

solution#4284
Skay9n wants to merge 3 commits intomate-academy:masterfrom
Skay9n:develop

Conversation

@Skay9n
Copy link

@Skay9n Skay9n commented Feb 23, 2026

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

I am requesting changes because several core task requirements are still not met and will likely cause the tests to fail. The main blockers are:

  1. Missing default exports / wrong imports

    • src/components/Article/Article.jsx, src/components/Header/Header.jsx, and src/components/Welcome/Welcome.jsx all use named exports instead of export default, but the task explicitly says: "Tests expect you to use export default for all the components."
    • In src/App.jsx, you import these components with named imports ({ Article }, { Welcome }, { Header }). After converting the components to default exports (for example export default function Header() { ... }), update the imports to default imports:
      import Header from './components/Header/Header';
      import Welcome from './components/Welcome/Welcome';
      import Article from './components/Article/Article';
  2. CSS not split per component [CHECKLIST ITEM #2]

    • src/App.css still contains styles for .welcome, .header, .article, and the navigation. The task requires: "CSS code should be split too" and "create separate folder per component".
    • Move these rules into component-level stylesheets, for example:
      • src/components/Welcome/Welcome.css for .welcome styles (and import it in Welcome.jsx)
      • src/components/Header/Header.css for .header styles
      • src/components/Article/Article.css for .article styles
      • A new src/components/Navigation/Navigation.css for nav-related styles
    • After moving, remove the component-specific rules from App.css, leaving only truly global styles if needed.
  3. Navigation not extracted into its own component

    • In src/components/Header/Header.jsx, the <nav>...</nav> markup is still inside Header. The description requires moving navigation into a separate Navigation component.
    • Create a Navigation component (e.g. src/components/Navigation/Navigation.jsx with export default function Navigation() { ... }), move the <nav> markup there, give it its own CSS file, and then import and render <Navigation /> inside Header.

Once you: (1) switch all components to default exports and fix the imports in App.jsx, (2) split the CSS into per-component files and clean up App.css, and (3) extract the navigation into a Navigation component, your solution should align well with the task requirements. You’ve already done good work structuring the basic components; addressing these points will complete the decomposition and make the project test-friendly.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

import './Article.css';
// Create an Article function returning the HTML of article block

export function Article() {

Choose a reason for hiding this comment

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

The task requires a default export for all components. This component is currently using a named export. You should add export default Article; at the end of the file.

src/App.jsx Outdated
Comment on lines 3 to 5
import { Article } from './components/Article/Article';
import { Welcome } from './components/Welcome/Welcome';
import { Header } from './components/Header/Header';

Choose a reason for hiding this comment

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

You import components using named imports ({ Article }, { Welcome }, { Header }) but the task states: "Tests expect you to use export default for all the components." Convert those components to default exports and update these imports to default imports (no braces). See description: "Tests expect you to use export default for all the components."

Comment on lines +10 to +12
<Welcome />
<Header />
<Article />

Choose a reason for hiding this comment

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

The app currently uses a global stylesheet rather than per-component styles. The task requires splitting CSS per component: move the .welcome, .header, .article, and navigation styles into their component folders (e.g. Welcome/Welcome.css, Header/Header.css, Article/Article.css) and import those styles from the component files instead of keeping everything in App.css. See the requirement: "CSS code should be split too." and review the existing App.css which contains these rules.


import './Header.css';
// Create a Header function returning the HTML of header block
export function Header() {

Choose a reason for hiding this comment

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

This file exports Header as a named export (export function Header) but the task states: "Tests expect you to use export default for all the components." Convert this to a default export (for example: export default function Header() { ... }). See the task description .

Comment on lines 8 to 18
<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>

Choose a reason for hiding this comment

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

The navigation markup is still inside Header (<nav>...</nav>). The description requires moving navigation into a Navigation component: "Move the navigation into a Navigation component (you may also create a folder for the Navigation component and move navigation there)." Extract the <nav> into its own component (with its own CSS) and import it into Header so the structure matches the task requirements .

@@ -1,5 +1,23 @@
// import a css file containig header styles

import './Header.css';

Choose a reason for hiding this comment

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

You import ./Header.css here, but the global App.css still contains .header rules. Ensure you move header-related styles from App.css into Header/ (Header.css) and remove them from App.css so CSS is split per component as required by the task. Current App.css shows .header defined there .


import './Welcome.css';
// Create a Welcome function returning the HTML of welcome block
export function Welcome() {

Choose a reason for hiding this comment

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

This file uses a named export (export function Welcome) but the task requires default exports: "Tests expect you to use export default for all the components." Convert to a default export (for example: export default function Welcome() { ... }).

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

I am approving your solution because all core requirements are implemented and nothing in the current code blocks functionality or task completion. You correctly split App into Welcome, Header, and Article components, each in its own folder with a default export, and you extracted Navigation into its own component that Header uses. CSS is cleanly separated per component (Welcome.css, Header.css, Article.css, Navigation.css), and App.css is reserved for global styles only, which matches the checklist expectation for [CHECKLIST ITEM #2]. There are no apparent syntax errors or broken imports/exports across the reviewed files.

The main follow-ups are process/tooling related rather than blockers: make sure you run npm test locally and fix any failing tests if they appear, ensure all new component files and CSS are committed to git, and update the DEMO link in your PR description to use your GitHub username as required. As a minor polish, you can remove any leftover instructional comments from the components to keep the codebase clean. Overall, this is a solid, well-structured implementation of the decomposition and styling requirements—well done.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

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