Skip to content

elad abutbul#1

Open
Elad-Abutbul wants to merge 1 commit intoColmanDevClubORG:mainfrom
Elad-Abutbul:elad-abutbul
Open

elad abutbul#1
Elad-Abutbul wants to merge 1 commit intoColmanDevClubORG:mainfrom
Elad-Abutbul:elad-abutbul

Conversation

@Elad-Abutbul
Copy link

No description provided.

Copy link
Member

@Tamir198 Tamir198 left a comment

Choose a reason for hiding this comment

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

Looks god overall

{/* Add button to navigate to /user/123 using useNavigate */}
<div style={{ marginTop: "20px" }}>
<h3>P{HOME_CONSTANTS.USING_USENAVIGATE}</h3>
<button onClick={() => navigate(ROUTE_CONSTANTS.USER_123)}>
Copy link
Member

Choose a reason for hiding this comment

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

I would extract this into a separate function just to make it a bit more readable

{/* Add Routes and Route components here */}
{/* Routes: /, /about, /contact, /user/:id, and catch-all for 404 */}
<div>App Component</div>
<Routes>
Copy link
Member

Choose a reason for hiding this comment

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

You could extract this into s Router component, because in the future you will have a lot of things inside the app file (like providers) and it will not be so easy to read

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