nodejs / nodejs.org

The Node.js® Website
https://nodejs.org
MIT License
6.15k stars 6.21k forks source link

feat: default transition (for colors) to `0.3s` #6726

Closed RedYetiDev closed 4 months ago

RedYetiDev commented 4 months ago

Description

This PR sets the default transition to 0.3s

Validation

To validate whether a transition exists, hover over some elements, and verify a 0.3s fade from their properties.

Check List

vercel[bot] commented 4 months ago

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

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview May 21, 2024 8:09pm
RedYetiDev commented 4 months ago

The only visual issue I am seeing is with the language selector.

AugustinMauroy commented 4 months ago

Note: SyntaxError: Unexpected identifier 'assert', might be a local Node.js issue

Nodejs website use V20 so the error should come from you node version

RedYetiDev commented 4 months ago

Note: SyntaxError: Unexpected identifier 'assert', might be a local Node.js issue

Nodejs website use V20 so the error should come from you node version

Good to note, thanks

ovflowd commented 4 months ago

Why is this a fix? What is it fixing? Could you please attach screenshots?

RedYetiDev commented 4 months ago

Why is this a fix?

It's a feature (feat)

What is it fixing?

N/A

Could you please attach screenshots?

Because it's a transitional effect, screenshots aren't possible, would you like a little video recording?

ovflowd commented 4 months ago

Why is this a fix?

It's a feature (feat)

What is it fixing?

N/A

Could you please attach screenshots?

Because it's a transitional effect, screenshots aren't possible, would you like a little video recording?

Sure; Please add more details explaining why is this needed, what is this improving and before/after videos of what is affecting.

AugustinMauroy commented 4 months ago

If I've understood correctly @RedYetiDev you're trying to avoid the flash between the ligh/dark theme?

RedYetiDev commented 4 months ago

If I've understood correctly @RedYetiDev you're trying to avoid the flash between the ligh/dark theme?

Sorry for the delay in my response, yes that is part of the reason, but also other things (like hover effects). I’ll make a video difference soon

RedYetiDev commented 4 months ago

New

screen-capture.webm

Old

screen-capture (1).webm

ovflowd commented 4 months ago

I see, I like the changes. But since this changes every page; Please ensure to test every page for inconsistencies or visual bugs this change might introduce.

github-actions[bot] commented 4 months ago
Lighthouse Results URL Performance Accessibility Best Practices SEO Report
/en 🟢 98 🟢 100 🟢 100 🟢 91 🔗
/en/about 🟢 100 🟢 100 🟢 100 🟢 91 🔗
/en/about/previous-releases 🟢 97 🟢 100 🟢 100 🟢 92 🔗
/en/download 🟢 94 🟢 100 🟢 100 🟢 91 🔗
/en/blog 🟢 99 🟢 100 🟢 100 🟢 92 🔗
github-actions[bot] commented 4 months ago

Unit Test Coverage Report

Lines Statements Branches Functions
Coverage: 91%
90.06% (589/654) 76.08% (175/230) 92.24% (119/129)

Unit Test Report

Tests Skipped Failures Errors Time
128 0 :zzz: 0 :x: 0 :fire: 6.695s :stopwatch:
RedYetiDev commented 4 months ago

LGTM; I understand the purpose of the changes now. But I'd recommend the author to discretionarily test the website (and all its pages, buttons, etc) to ensure that this change does not introduce issues.

Currently, it introduces one issue (with a weird transition for the language selector), but I'll look for more, than fix what needs to be done

AugustinMauroy commented 4 months ago

https://github.com/nodejs/nodejs.org/assets/97875033/a690b735-7b5a-47f7-8782-b79f000f50e3

Also there this strange behavior

RedYetiDev commented 4 months ago

The dropdowns are experiencing strange behavior, I need to fix that.

Once I do that I'll also move to tailwind

ovflowd commented 4 months ago

The dropdowns are experiencing strange behavior, I need to fix that.

Once I do that I'll also move to tailwind

I appreciate your thoroughness!