Skip to content
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

[nit] Select ARM64 by default in the dropdown if user is visiting from M1 #6364

Closed
trivikr opened this issue Feb 29, 2024 · 6 comments · Fixed by #6439
Closed

[nit] Select ARM64 by default in the dropdown if user is visiting from M1 #6364

trivikr opened this issue Feb 29, 2024 · 6 comments · Fixed by #6439
Labels
good first issue Issues for newcomers help wanted website redesign Issue/PR part of the Node.js Website Redesign

Comments

@trivikr
Copy link
Member

trivikr commented Feb 29, 2024

Enter your suggestions in details:

Is your feature request related to a problem? Please describe.

Not a big issue, but I was browsing new redesigned website from M1 (on ARM architecture) and noticed that X64 was selected in the dropdown by default for prebuilt installer and binaries

https://nodejs-org-git-meta-remove-legacy-website-code-openjs.vercel.app/en/download

Prebuilt Installer prebuilt-installer
Prebuilt Binaries prebuilt-binaries

Describe the solution you'd like

Select the architecture of visitors machine, if it's detectable from JavaScript.

For example, it's detectable from experimental await navigator.userAgentData.getHighEntropyValues(['architecture']) API call.

The website uses the experimental API to get bitness

const entropyValues = navigator.userAgentData.getHighEntropyValues([
'bitness',
]);

Describe alternatives you've considered

N/A, as the user can explicitly select ARM option from the dropdown even if X64 is selected by default.

@trivikr
Copy link
Member Author

trivikr commented Feb 29, 2024

The architecture might not matter in the dropdown for macOS, as both binaries link to the same .pkg installer.

@ovflowd
Copy link
Member

ovflowd commented Mar 2, 2024

Enter your suggestions in details:

Is your feature request related to a problem? Please describe.

Not a big issue, but I was browsing new redesigned website from M1 (on ARM architecture) and noticed that X64 was selected in the dropdown by default for prebuilt installer and binaries

nodejs-org-git-meta-remove-legacy-website-code-openjs.vercel.app/en/download

Prebuilt Installer
Prebuilt Binaries

Describe the solution you'd like

Select the architecture of visitors machine, if it's detectable from JavaScript.

For example, it's detectable from experimental await navigator.userAgentData.getHighEntropyValues(['architecture']) API call.

The website uses the experimental API to get bitness

const entropyValues = navigator.userAgentData.getHighEntropyValues([
'bitness',
]);

Describe alternatives you've considered

N/A, as the user can explicitly select ARM option from the dropdown even if X64 is selected by default.

Feel free to create a PR or patch to use the right bitness. We use the getBitness in that button but I believe the Bitness doesn't say the architecture (i.e. ARM)

Because the architecture is ARM and the bitness is 64. So, technically, the result is correct.

@ovflowd
Copy link
Member

ovflowd commented Mar 2, 2024

Pretty much an idea for a PR is to introduce the "getArchitecture" utility and embed it on our util that detects the UserOS (https://github.com/nodejs/nodejs.org/blob/main/hooks/react-client/useDetectOS.ts)

So you could add an extra field "architecture" that would then be used on our BitnessDropdown (

import { useDetectOS } from '@/hooks/react-client';
)

(Which is actually an architecture + bitness dropdown...)

@trivikr
Copy link
Member Author

trivikr commented Mar 4, 2024

I added good first issue and website redesign labels if anyone else wants to give it a try.

@trivikr trivikr added good first issue Issues for newcomers website redesign Issue/PR part of the Node.js Website Redesign labels Mar 4, 2024
@Vadimchesh
Copy link

I can take it

rhlshah added a commit to rhlshah/nodejs.org that referenced this issue Mar 4, 2024
@rhlshah
Copy link
Contributor

rhlshah commented Mar 4, 2024

I have fixed the issue and have also created a PR. Please have a look

github-merge-queue bot pushed a commit that referenced this issue Mar 6, 2024
* Fixed Issue #6364

* Fix arm64 select by default v2

* Fix arm64 select by default v2

* Update util/__tests__/getUserBitnessByArchitecture.test.mjs

Co-authored-by: Brian Muenzenmeyer <brian.muenzenmeyer@gmail.com>
Signed-off-by: Rahil Shah <45811662+rhlshah@users.noreply.github.com>

* Update util/getUserBitnessByArchitecture.ts

Co-authored-by: Brian Muenzenmeyer <brian.muenzenmeyer@gmail.com>
Signed-off-by: Rahil Shah <45811662+rhlshah@users.noreply.github.com>

* Update getUserBitnessByArchitecture.ts

Signed-off-by: Claudio W <cwunder@gnome.org>

* chore: fixed lint

---------

Signed-off-by: Rahil Shah <45811662+rhlshah@users.noreply.github.com>
Signed-off-by: Claudio W <cwunder@gnome.org>
Co-authored-by: Brian Muenzenmeyer <brian.muenzenmeyer@gmail.com>
Co-authored-by: Claudio W <cwunder@gnome.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues for newcomers help wanted website redesign Issue/PR part of the Node.js Website Redesign
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants