Skip to content

add task solution#4304

Open
nikassafronovs wants to merge 2 commits intomate-academy:masterfrom
nikassafronovs:develop
Open

add task solution#4304
nikassafronovs wants to merge 2 commits intomate-academy:masterfrom
nikassafronovs:develop

Conversation

@nikassafronovs
Copy link

No description provided.

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

Good work on decomposing the application into Welcome, Header, and Article components. However, some changes are needed to fully meet the task requirements. The main issue is that the decomposition is incomplete. The Header component and its CSS file still contain the logic and styles for the navigation. Please create a separate Navigation component for the navigation block, moving the relevant JSX and CSS into new Navigation.jsx and Navigation.css files. Additionally, your Article, Header, and Welcome components use both named and default exports, which is redundant. Please remove the named export keyword from the component declarations and use only the export default at the end of each file for consistency. Once these changes are made, your solution will be perfect!


✨ 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 const Article = () => (

Choose a reason for hiding this comment

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

While this works, having both a named export (export const) and a default export (on line 62) for the same component is redundant. The task requires a default export, which you have. It's a good practice to stick to a single export style for components. You can improve this by removing the export keyword here, changing the line to const Article = () => (.

import './Header.css';

// Create a Header function returning the HTML of header block
export const Header = () => (

Choose a reason for hiding this comment

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

Similar to the Article component, you have both a named export here and a default export on line 20. The task requires a default export, so the named export is unnecessary. It's better to stick to one export type for consistency. You can remove the export keyword from this line.

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;
}

Choose a reason for hiding this comment

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

These styles are for the navigation block. Based on the task description to split components by CSS blocks, these styles should be moved to a new Navigation.css file, corresponding to a new Navigation component. The task description explicitly suggests this as a possible improvement.

import './Welcome.css';

// Create a Welcome function returning the HTML of welcome block
export const Welcome = () => (

Choose a reason for hiding this comment

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

While this works, having both a named export (export const) and a default export for the same component is redundant. The task requires a export default. It's better practice to stick to one export method per component file. You can remove the export keyword here and just define const 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

Excellent work on this submission! I am happy to approve your solution. You've done a fantastic job of addressing all the feedback from the previous review. The application is now correctly decomposed into smaller, reusable components, including the new Navigation component, and each component has its own dedicated styling. The code is clean, well-structured, and fully meets all the requirements of the task. 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