Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ft/creating-dynamic-navbar #17

Merged
merged 14 commits into from
Apr 15, 2024
Merged

Ft/creating-dynamic-navbar #17

merged 14 commits into from
Apr 15, 2024

Conversation

kareraolivier
Copy link
Collaborator

updating the header by making content dynamic

@kareraolivier kareraolivier self-assigned this Mar 18, 2024
@kareraolivier kareraolivier added documentation Improvements or additions to documentation In-progress This pr is in still in progress ready for review The pr is ready for reviews and removed documentation Improvements or additions to documentation In-progress This pr is in still in progress labels Mar 18, 2024
Copy link
Collaborator

@AdelineA AdelineA left a comment

Choose a reason for hiding this comment

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

Looks good.

Comment on lines 30 to 35
<Image
src="./images/smallArrow.png"
alt="arrow"
width={100}
height={100}
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest using the svg for arrows instead of using images.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment on lines 44 to 49
<Image
src="./images/smallArrow.png"
alt="arrow"
width={100}
height={100}
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here .

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment on lines 51 to 56
<Image
src="./images/smallArrow.png"
alt="arrow"
width={100}
height={100}
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@AdelineA AdelineA left a comment

Choose a reason for hiding this comment

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

Great Job!

@AdelineA AdelineA added reviewed There is comment that require more attention and removed ready for review The pr is ready for reviews labels Mar 19, 2024
link: "/case-studies-and-core-tools",
title: "Level of Co-Design Practice",
},
];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you put this file in utils folder and give it .ts extension as it is not a tsx file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

next.config.js Outdated
@@ -4,6 +4,9 @@
const nextConfig = {
reactStrictMode: false,
output: "export",
images: {
unoptimized: true,
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

why optimize is disabled?

Copy link
Collaborator Author

@kareraolivier kareraolivier Mar 20, 2024

Choose a reason for hiding this comment

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

we need to disable optimization becouse we added output:"export", we can't run project local if we do not disable it.
and we need output:"export" so that we can build the project for gitpages


const HorzontalArrow = () => {
return (
<div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This div is not needed here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@kareraolivier kareraolivier force-pushed the ft/creating-dynamic-navbar branch 2 times, most recently from 6f1491f to 1d4d644 Compare March 20, 2024 14:18
@kareraolivier kareraolivier requested a review from NNesta March 21, 2024 06:36
@kareraolivier kareraolivier force-pushed the ft/creating-dynamic-navbar branch from 1d4d644 to 7d6b6ff Compare March 21, 2024 06:51
@kareraolivier kareraolivier force-pushed the ft/creating-dynamic-navbar branch from 047b24f to 87b1058 Compare March 21, 2024 12:20
Copy link
Collaborator

@NNesta NNesta left a comment

Choose a reason for hiding this comment

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

LGTM

@kareraolivier kareraolivier requested a review from musayann March 22, 2024 14:35
@musayann musayann changed the base branch from dev to main April 15, 2024 14:41
@kareraolivier kareraolivier force-pushed the ft/creating-dynamic-navbar branch from a3edf4f to 0196030 Compare April 15, 2024 14:51
@musayann musayann merged commit 4a32cb7 into main Apr 15, 2024
@kareraolivier kareraolivier linked an issue Apr 15, 2024 that may be closed by this pull request
@musayann musayann deleted the ft/creating-dynamic-navbar branch April 15, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reviewed There is comment that require more attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ft/Creating dynamic navbar
4 participants