Skip to content

add task solution#4328

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

add task solution#4328
katrysyar wants to merge 3 commits intomate-academy:masterfrom
katrysyar:develop

Conversation

@katrysyar
Copy link
Copy Markdown

Copy link
Copy Markdown

@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

Great job on decomposing the application into smaller components! The overall structure is well-organized. However, a couple of adjustments are needed to fully meet the task requirements.

Here are the main points to address:

  1. Create a Navigation Component: The navigation section is still part of the Header component. As mentioned in the task, please create a separate Navigation component in its own folder. Move the <nav> block from Header.jsx and its related styles from Header.css into the new component's files. Then, render the new <Navigation /> component inside the Header.

  2. Correct Component Exports and Imports: Your components currently have both named and default exports (e.g., export function Welcome(...) and export default Welcome). The task requires using only export default. Please remove the export keyword from your function declarations and rely solely on the default export at the end of the file. This will also require you to change the imports in App.jsx from named imports like { Welcome } to default imports like import Welcome from './components/Welcome/Welcome';.

Once these changes are made, your solution will be perfect. Keep up the great work!


✨ 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

Comment on lines +21 to +34
.navigation__link {
display: inline-block;
outline: none;
text-decoration: none;
opacity: 0.7;
padding: 0 0.5em;
color: black;
transition: opacity 0.2s ease-in-out;
}

.navigation__link:hover,
.navigation__link:focus {
opacity: 1;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to the task description, you should create a separate Navigation component. These navigation-related styles should be moved to a corresponding Navigation.css file inside the Navigation component's folder.

src/App.jsx Outdated
Comment on lines +4 to +6
import { Welcome } from './components/Welcome/Welcome';
import { Header } from './components/Header/Header';
import { Article } from './components/Article/Article';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires using export default for all components. While your component files do have default exports, you are using named imports here ({ Welcome }). It's conventional to use default imports when a module provides a default export. Please change this to a default import: import Welcome from './components/Welcome/Welcome';. The same applies to the Header and Article imports.

import './Article.css';

// Create an Article function returning the HTML of article block
export function Article() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires using export default for all components. By adding the export keyword here, you are creating a named export in addition to the default export at the end of the file. This is redundant. Please remove the export keyword from this function declaration to ensure there is only one default export.

import './Header.css';

// Create a Header function returning the HTML of header block
export function Header() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires using export default for all components. By adding the export keyword here, you are creating a named export in addition to the default export at the end of the file. This is redundant. Please remove the export keyword from this function declaration to ensure there is only one default export.

Comment on lines +7 to +17
<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>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task description suggests creating a separate Navigation component for this block of code. You should create a Navigation component in its own folder, move this <nav> element and its contents there, and then import and render the Navigation component here.

import './Welcome.css';

// Create a Welcome function returning the HTML of welcome block
export function Welcome() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires using export default for all components. By adding the export keyword here, you are creating a named export in addition to the default export at the end of the file. This is redundant. Please remove the export keyword from this function declaration to ensure there is only one default export.

Copy link
Copy Markdown

@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

Great job on creating the Navigation component and splitting the CSS files! The project structure is much cleaner now.

However, there's one critical requirement from the task that needs to be addressed before this can be approved. The task requires using only export default for all components. The Navigation component is still using a named export.

To fix this, please make the following changes:

  1. In src/components/Navigation/Navigation.jsx, remove the export keyword from the function declaration. It should be function Navigation() { ... }, not export function Navigation() { ... }.
  2. In src/components/Header/Header.jsx, update the import statement to use a default import: change import { Navigation } from '../Navigation/Navigation'; to import Navigation from '../Navigation/Navigation';.

You're very close to finishing! Just this one change is needed to meet all the requirements.


✨ 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 './Header.css';

// Create a Header function returning the HTML of header block
import { Navigation } from '../Navigation/Navigation';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires using default imports for all components. This should be import Navigation from '../Navigation/Navigation';.

You'll also need to ensure that the Navigation.jsx file is correctly set up with only a default export, as mentioned in the previous review.

@@ -0,0 +1,19 @@
import './Navigation.css';

export function Navigation() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This named export should be removed. The task requires using only export default. Please remove the export keyword from the function declaration and rely on the export default Navigation; at the end of the file.

Copy link
Copy Markdown

@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

Excellent work on this submission! You've successfully addressed all the feedback from the previous review. The project now correctly uses export default for all components, and the component decomposition into separate folders with their own CSS is done perfectly. The project structure is clean and well-organized. Your solution is approved, great job!


✨ 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