Skip to content

fix height dropdown #7989

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

abinadiS
Copy link

Description

Adjust dropdown max-height from 48 to 56
#7969

Validation

Related Issues

Check List

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

Copy link

vercel bot commented Jul 16, 2025

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

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Jul 16, 2025 10:53pm

Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

IDK what should we do. Like if we add new entry in download select do we will increase size ?

@MikeMcC399
Copy link
Contributor

MikeMcC399 commented Jul 16, 2025

@abinadiS

I see that your PR is still in draft, so maybe you are not ready for comment yet, but anyway I will ask:

  • What happens if more items are added to the list. Will this still work?
  • Is it necessary to have the maximum height restriction or could this be removed completely? So it would look like this:
image
  • If maximum height is removed would it adversely affect other dropdowns on the website? Edit: answer - yes, as the same class is also used for each of the dropdowns, including the version list, which looks very untidy if all the versions are displayed.

I wonder if the dropdown for Release.InstallationMethodDropdown needs its own class so that the list length for this can be handled differently?

Also your commit message would need to comply with https://github.com/nodejs/nodejs.org/blob/main/docs/code-style.md#commit-guidelines so, you would need to change to fix: height dropdown, for example.

Finally, if you write closes #7969 in your original post, it will automatically link to the issue.

@abinadiS
Copy link
Author

Yes, it's because it shares the same class, and that also affects the versions dropdown menu. For it to only affect that specific element, it would have to be done with a 'prop' (property), and that would involve more changes. I just tried to make it as simple as possible and, seeing that it didn't significantly affect the versions list, I decided to submit it like that. I can adjust it so that everything displays correctly, even if more items are added to that list.

@MikeMcC399
Copy link
Contributor

@abinadiS

Thanks for explaining! It's definitely an improvement from the current state. I'm just a user of this site, not a front-end developer or a site owner, so I think the site maintainers will need to give their opinion on the change.

If you are ready for the PR to be reviewed, you can take it out of draft state.

@abinadiS
Copy link
Author

@MikeMcC399 is very important user opinions, thanks for your comment, I can test what you say to find the easiest way and expect approval.

@abinadiS
Copy link
Author

@MikeMcC399
Captura de pantalla 2025-07-16 a la(s) 5 59 54 p m

Thanks for your comment, I made some adjustments.

@abinadiS abinadiS marked this pull request as ready for review July 16, 2025 23:03
@abinadiS abinadiS requested a review from a team as a code owner July 16, 2025 23:03
@MikeMcC399
Copy link
Contributor

@abinadiS

This is an excellent improvement from a user perspective! 🎉 Thank you very much!

@avivkeller
Copy link
Member

@nodejs/nodejs-website what if we just made them all bigger

@MikeMcC399
Copy link
Contributor

@avivkeller

what if we just made them all bigger

That would be a step back in the evolution of this PR. I tried doing that and as I mentioned in #7989 (comment) the dropdown list for versions becomes very long and untidy, displaying additional end-of-life versions to download.

This is off-topic for this PR, but I'll just mention it here: as an enhancement, the version list could display only supported versions by default and had a toggle to show all versions.

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.

4 participants