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

Added bold on current page #175

Merged
merged 3 commits into from
Mar 14, 2024
Merged

Conversation

Tamir198
Copy link
Contributor

Fixed #173

Added missing bold on the current page libnk :

image
image

Copy link

vercel bot commented Mar 12, 2024

@Tamir198 is attempting to deploy a commit to a Personal Account owned by @Maakaf on Vercel.

@Maakaf first needs to authorize it.

<li className="cursor-pointer body-roman">
<Link href={LINKS.PROJECTS} className='transition duration-300 group'>הפרויקטים
<span className="block max-w-0 group-hover:max-w-full transition-all duration-500 h-0.5 bg-sky-600"/>
<li className={`cursor-pointer body-roman ${currentPage === 'projects' ? 'font-bold' : ''}`}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe using an array for future use if there will be changes in the nav. All the styles here is duplicated

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe using an array for future use if there will be changes in the nav. All the styles here is duplicated

makes sense....want me to assign you this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Tamir198 Is already on it no?

Copy link
Collaborator

Choose a reason for hiding this comment

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

He did start but was not assigned its an oversight on my end.
he is now :D
thanks for looking into this!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean an array that will contain the strings? something else?

Generally, we have a lot of styles duplicated all over the project.
In my opinion, It will be better to use layer to create custom classes for generic purposes (for example, card, navItem etc...) together with generic components.

If the same style repeats itself it can be a good indication that this part can be extracted into a custom component.

I think that think is a project level thing, related to the project architecture.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about array of the items in the header and then iterate over it inside the header with the same style

@Darkmift Darkmift merged commit 4565925 into Maakaf:main Mar 14, 2024
3 of 4 checks passed
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.

header indication acording to page
3 participants