fix: Certain languages break the max height of the search bar #6841

[nilkhankari]

Description

Increased width of search button from 13rem to 15rem

Validation

Related Issues

Fixes: https://github.com/nodejs/nodejs.org/issues/6841

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]]

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

Name	Status	Preview	Updated (UTC)
nodejs-org	✅ Ready (Inspect)	Visit Preview	Jul 4, 2024 4:28pm

[bmuenzenmeyer]

Two things

The keyboard shortcut still wraps. This is perhaps a separate issue, but we are in this section of the codebase here and it seems relevant to improve.

There is a viewport range (for me, on WIndows, Chrome, default zoom, fr locale) between1024px and 1114px where the site has a horizontal scrollbar.

I'd imagine we want to avoid that.

[nilkhankari]

Hi @bmuenzenmeyer , Thanks for the review. I am thinking of setting the width of the search button to 16rem. This will resolve the first issue. Regarding the second issue, I am considering reducing the gap and padding to 1rem. Do you have any suggestions on this proposed solution?

[ovflowd]

Hi @bmuenzenmeyer , Thanks for the review. I am thinking of setting the width of the search button to 16rem. This will resolve the first issue. Regarding the second issue, I am considering reducing the gap and padding to 1rem. Do you have any suggestions on this proposed solution?

I think this is fine. Can you go ahead with it? (Dont forget to test different viewports, including mobile ones)

[nilkhankari]

Validation with new changes

For French language

For Portuguese

For English

[nilkhankari]

@bmuenzenmeyer @ovflowd Could you please review this.

[nilkhankari]

@ovflowd Could you please suggest what we should do with this PR?

[ovflowd]

@ovflowd Could you please suggest what we should do with this PR?

Sorry for the delay! I did not get pinged here. Let's try @nazarepiedady's approach, as that makes sense. I'm worried with the tablet/small screen breakpoints, but we could add a max-width as a last resort and use padding here instead of setting widths.

Could you try that?

[ovflowd]

It looks like @nazarepiedady made a PR already https://github.com/nodejs/nodejs.org/pull/6904

[github-actions[bot]]

Unit Test Coverage Report

Unit Test Report

Tests	Skipped	Failures	Errors	Time
131	0 :zzz:	0 :x:	0 :fire:	5.288s :stopwatch:

[nilkhankari]

It looks like @nazarepiedady made a PR already #6904

@ovflowd okay. Please suggest now what to do with this PR.

[ovflowd]

It looks like @nazarepiedady made a PR already #6904

@ovflowd okay. Please suggest now what to do with this PR.

I assume we'd proceed with Nazare's PR, but I'd keep this PR open until that gets merged. But feel free to close it, also.

[nazarepiedady]

@ovflowd, @bmuenzenmeyer, I believe we can close this pull request since the pull request I did is already merged.

I also would like to thank you, @nilkhankari, for your contribution because some of the updates that I did on my pull request were based on your pull request and ideas.

[mikeesto]

Closing as https://github.com/nodejs/nodejs.org/pull/6904 landed. Thanks everyone