Enhancement: added navItem hover

[mAmineChniti]

Description

Just a very simple change, added a couple of lines of css to have the top navBar that has Learn, About, etc.. have the same hover effect as the footer bar

Validation

Related Issues

This was suggested in:
#6752

Check List

• I have read the Contributing Guidelines and made commit messages that follow the guideline.
• I have run npm run format to ensure the code follows the style guide.
• I have run npm run test to check if all tests are passing.
• I have run npx turbo build to check if the website builds without errors.
• I've covered new added functionality with unit tests if necessary.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
nodejs-org	✅ Ready (Inspect)	Visit Preview	Jun 14, 2024 1:17pm

[mikeesto]

Thanks for the PR. Personally I think this is a nice addition - I'm interested to hear others' thoughts

[AugustinMauroy]

+1 for theses change, I remember we already have discussion about hover effect but I didn't know in which pr.

[ovflowd]

I thought we discussed it before and decided not to do this. What changed now? 🤔

[AugustinMauroy]

I thought we discussed it before and decided not to do this. What changed now? 🤔

I used to agree with that. And if I remember correctly last time the proposal was that the hover effect should be the same as the active style for navigation, which was for ‘heavy’.

[canerakdas]

I do not think the links in the header and footer navigation items are buttons, therefore I do not think it is right for these elements to behave like buttons in the hover state

Here is why;
https://a11y-101.com/design/button-vs-link

Maybe changing the text color would be better on hover, I'm not 100% against it as the current solution only works on hover 🙈

[ovflowd]

Maybe changing the text color would be better on hover, I'm not 100% against it as the current solution only works on hover 🙈

Agreed, and that's what I said on the last PR. This should at maximum change the text on hover, but this is a link, not a button.

[AugustinMauroy]

and why not just underline it when hover it's discreet and allow user to know there are possible interaction.

[mAmineChniti]

The footer links have this same effect on hover, should i go with underline or text color change and should i do it for the top navBar or both it and the footer?

[canerakdas]

and why not just underline it when hover it's discreet and allow user to know there are possible interaction.

Since we use horizontal lines to divide the design into sections(header, content, footer), I think the use of underlines in the texts would be overkill / retro look 👀 Wouldn't it be enough to indicate it with text color and cursor pointer?

[mAmineChniti]

@canerakdas I made the text color change to the nodejs green (text-green-400) on hover in both dark and light mode, its visible in both modes

[mAmineChniti]

I'm still very much a beginner but i have a take on this that i'm not sure if valid or not so please correct me on this:
I think that the contrast between the previous color and the color on hover is much more important than the contrast between the new color and the background, having the text change from black to a lighter color that still looks visible enough with a white background draws the eyes better than going from black to another dark color that the user won't pick up on, the darker color has really good contrast with the bg but is not different enough from the previous color (black) so the user won't pick up on it that well

[ovflowd]

I think that the contrast between the previous color and the color on hover is much more important than the contrast between the new color and the background, having the text change from black to a lighter color that still looks visible enough with a white

Unfortunately, not. For accessibility standards, the contrast on the background color is way more important. There are a bunch of reasons why, and I believe you could study those, as these are standard UX and accessibility practices.

Maybe this blog post can help you out :) https://medium.com/@think_ui/why-color-contrast-is-not-as-black-and-white-as-it-seems-94197a72b005

[bmuenzenmeyer]

LGTM

[bmuenzenmeyer]

@ovflowd I know you are busy and feel free to ignore, but curious if you are accepting of this change. We've gone around and around, but want to make sure you had a chance to review it too.

if I don't hear from you, I will merge tomorrow

[ovflowd]

Appreciate tagging me. Honestly speaking I'm a -1 for the current proposal (how the PR stands)

But I won't go out my way to block the PR if I get reassurance the concerns Caner and I shared initially get addressed in a follow-up.

I understand the why's, but these are not buttons, but links.

Buttons do actions. Links do navigation. Anyhow, feel free to merge.

[AugustinMauroy]

I tend to agree with Claudio, but with these changes the footer and header will be in line.
I'd be in favour of merging this pr and then opening an issue like ‘difference between link and action isn't clear on header/footer’.

[ovflowd]

It's less about being clear and more about being accessible and compliant imo

[bmuenzenmeyer]

It's less about being clear and more about being accessible and compliant imo

it is keyboard accessible.

the heart of the linked resource suggests that actual controls, like input elements shouldn't look like links.

[github-actions]

Lighthouse Results

URL	Performance	Accessibility	Best Practices	SEO	Report
/en	🟢 97	🟢 100	🟢 96	🟢 91	🔗
/en/about	🟢 100	🟢 100	🟢 96	🟢 91	🔗
/en/about/previous-releases	🟢 96	🟢 100	🟢 100	🟢 92	🔗
/en/download	🟢 98	🟢 100	🟢 96	🟢 91	🔗
/en/blog	🟢 99	🟢 100	🟢 96	🟢 92	🔗

[github-actions]

Unit Test Coverage Report

Lines	Statements	Branches	Functions
	90.67% (593/654)	76.08% (175/230)	94.57% (122/129)

Unit Test Report

Tests	Skipped	Failures	Errors	Time
131	0 💤	0 ❌	0 🔥	5.254s ⏱️

[bmuenzenmeyer]

@mAmineChniti can you sync your fork with nodejs.org/main ?

[mAmineChniti]

@mAmineChniti can you sync your fork with nodejs.org/main ?

Done.

[bmuenzenmeyer]

thanks @mAmineChniti for coming with us on this journey. I hope you find more ways to contribute to the project!