doc: allow downloading wordmark & logo assets on right-click #1278
Conversation
|
@AmirAliAzimloo is attempting to deploy a commit to the 47ng Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
franky47
left a comment
franky47
left a comment
There was a problem hiding this comment.
Thanks, could you break down the download code into dedicated functions outside the component body please? This makes a huge wall of text that's hard to maintain.
| githubUrl: 'https://github.com/47ng/nuqs', | ||
| nav: { | ||
| title: <NuqsWordmark className="ml-2 text-xl" />, | ||
| title: <NuqsWordmark className="ml-2 text-xl" hasDownloader={true} />, |
There was a problem hiding this comment.
suggestion: use a shorthand boolean here
| title: <NuqsWordmark className="ml-2 text-xl" hasDownloader={true} />, | |
| title: <NuqsWordmark className="ml-2 text-xl" hasDownloader />, |
|
|
||
| const handleCopyLogoSvg = async () => { | ||
| try { | ||
| const textPromise = fetch(LOGO_ADDRESS_URL).then(response => |
There was a problem hiding this comment.
note: We could get the SVG text in a server component, pass the Promise of its contents down to the client component and consume it with use, no need for a runtime fetch here.
franky47
changed the title
…ature/logo-downloader
|
@franky47 Hi, I’ve completed all the requested changes. When you have a moment, could you please review them again? Thanks. |
| className="fixed z-50" | ||
| style={{ left: position.x, top: position.y }} | ||
| > | ||
| <div className="w-[240px] overflow-hidden rounded-xl border border-gray-200/80 bg-white shadow-xl backdrop-blur-sm dark:bg-black"> |
There was a problem hiding this comment.
issue: The border-gray-200/80 gives a hard border in dark mode, use border-border/80 for a better effect.
| const computedStyle = window.getComputedStyle(originalPath) | ||
| const fillColor = computedStyle.fill |
There was a problem hiding this comment.
note: Somehow this returns colors in rgb(rrr, ggg, bbb), is there a way to output them as hex instead (more compact) ?
| } | ||
|
|
||
| export const handleCopyWordmarkPng = async () => { | ||
| const wordmark = document.getElementById( |
There was a problem hiding this comment.
issue: One problem with going via the DOM is that we get different results whether we use this menu in light mode or dark mode.
While it might make sense, I don't think it's intuitive that you need to change the whole site's theme to get a different result.
suggestion: We should have light & dark variants for both SVG & PNG (I have added /ico.dev.svg for the light version of the logo).
| 'use client' | ||
|
|
||
| import { FC, use, useCallback, useEffect, useRef, useState } from 'react' | ||
| import { SiGooglephotos, SiSvg } from '@icons-pack/react-simple-icons' |
There was a problem hiding this comment.
issue: The google photos icon doesn't work here, could we use the Image icon from lucide instead?
Same for SVG, I don't think a lot of people know the official SVG icon. A double chevron (code from lucide) might work better.
| ref={menuRef} | ||
| className="fixed z-50" | ||
| style={{ left: position.x, top: position.y }} | ||
| > |
There was a problem hiding this comment.
issue: We should use shadcn for custom UI components, as they are accessible by design (arrow navigation & focus management) and follow the design system.
…ature/logo-downloader
…ature/logo-downloader
|
@franky47 Thanks, dear. I implemented the code structure and logic based on our discussion above. Could you please review them? Thanks. |
2 participants
Implements logo and wordmark download functionality as described in this issue.
Changes :
Closes #1253.