nodejs / nodejs.org

The Node.js® Website

https://nodejs.org

MIT License

6.15k stars 6.22k forks source link

fix: overflow issue on mobile GlowingBackdrop #6683

Closed canerakdas closed 5 months ago

canerakdas commented 5 months ago

Description

This PR aims to solve GlowingBackdrop overflow for the mobile resolution by limiting the container.

Validation

A glowing backdrop in the preview should not be wider than the content

Check List

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

vercel[bot] commented 5 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 1, 2024 1:23pm

github-actions[bot] commented 5 months ago

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

github-actions[bot] commented 5 months ago

Unit Test Coverage Report

Lines	Statements	Branches	Functions
	90.04% (588/653)	76.08% (175/230)	92.18% (118/128)

Unit Test Report

Tests	Skipped	Failures	Errors	Time
128	0 :zzz:	0 :x:	0 :fire:	5.816s :stopwatch:

canerakdas commented 5 months ago

I think there is also a problem with the centering of the gradient 🤔

bmuenzenmeyer commented 5 months ago

odd - i wonder if this was a regression

ovflowd commented 5 months ago

odd - i wonder if this was a regression

Probably a regression for when we switched to use a React Component as the Backdrop? Instead of a background CSS attribute?

ovflowd commented 5 months ago

~I'm fast tracking this as it seems quite an ugly bug.~ Actually, not, have a pending question.

ovflowd commented 5 months ago

@canerakdas even if the glowing part is centered now, apparently the Hexagon itself is not. I have the feeling instead of using overflow-hidden we should actually limit the size of the Hexagon/center it?

canerakdas commented 5 months ago

odd - i wonder if this was a regression

It looks like a regression, I noticed it while randomly browsing the website 🤔

canerakdas commented 5 months ago

even if the glowing part is centered now, apparently the Hexagon itself is not. I have the feeling instead of using overflow-hidden we should actually limit the size of the Hexagon/center it?

I have updated the svg size according to the aspect ratio 🎉