-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
Docs: add n as community download option #7498
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment was marked as outdated.
This comment was marked as outdated.
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shadowspawn waiting for you to update the copies and then we're good to go!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Lighthouse Results
|
What copies? |
My only nitpick/concern is that the MacOS close/min/max buttons don't appear to be contained within the logo's content, and there appears to be a white border (https://snapshots.chromatic.com/snapshots/64c7d71358830e9105808652-67df1c3434d85d3bf91872e0/capture-b3e22694.png). If this is intentional, ignore this comment, however if it's not intentional, maybe some slight adjustments could improve this? i.e. <svg
width="100%"
height="100%"
viewBox="0 0 64 64"
xmlns="http://www.w3.org/2000/svg"
>
<rect width="64" height="64" rx="8" fill="#121212"/>
<circle cx="10" cy="10" r="4" fill="#ff5f56"/> <!-- Close -->
<circle cx="20" cy="10" r="4" fill="#ffbd2e"/> <!-- Minimize -->
<circle cx="30" cy="10" r="4" fill="#27c93f"/> <!-- Maximize -->
<text x="16" y="44" font-family="monospace" font-size="32" font-weight="bold" fill="#00ff7f">
$n
</text>
</svg> |
The white border was intended so icon identifiable on a black background. The buttons positions were not done carefully, good call on questioning those. I will take a look when back at a computer. |
Improved button positions in icon following suggestion. |
Thanks! Looks good (https://64c7d71358830e9105808652-yhorhypche.chromatic.com/?path=/story/design-system--platform-logos&globals=viewport:large)! |
Hey, @shadowspawn! We recently merged some changes, if you could rebase please. I'm here to help you if needed, but once that's done I'd be happy to merge this! |
55570e2
to
e4235a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I manually rebased PR (automated rebase went badly). I noticed Volta was missing from |
Description
Add n as a download method for Node.js.
Related issues in n and n-install projects:
Validation
Related Issues
Check List
npm run format
to ensure the code follows the style guide.npm run test
to check if all tests are passing.npx turbo build
to check if the website builds without errors.