feat: add copy url button on api key dialog, close 1072#1079
feat: add copy url button on api key dialog, close 1072#1079looplj merged 1 commit intorelease/v0.9.xfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant usability improvement to the API key management interface. Users can now quickly copy the base URL for different SDKs directly from the API key view dialog, streamlining the process of configuring API clients. This change enhances the developer experience by providing immediate access to crucial setup information, reducing manual effort and potential errors. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a helpful "Copy Base URL" button to the API key view dialog. The implementation is solid, but I've identified a couple of opportunities to enhance component robustness and user interface consistency. My recommendations include using a useEffect hook for safer timer management and adopting the CheckIcon for the copy success state to align with existing patterns in the application.
| @@ -1,22 +1,46 @@ | |||
| import { useState, useEffect, useMemo } from 'react'; | |||
| import { Copy, Eye, EyeOff, AlertTriangle } from 'lucide-react'; | |||
| import { Copy, Eye, EyeOff, AlertTriangle, Link } from 'lucide-react'; | |||
There was a problem hiding this comment.
To improve UI consistency, it's better to use a CheckIcon to indicate a successful copy operation. To do so, you'll need to import it from lucide-react.
| import { Copy, Eye, EyeOff, AlertTriangle, Link } from 'lucide-react'; | |
| import { Copy, Eye, EyeOff, AlertTriangle, Link, CheckIcon } from 'lucide-react'; |
| const handleCopy = async () => { | ||
| await navigator.clipboard.writeText(baseUrl); | ||
| setIsCopied(true); | ||
| toast.success(t('apikeys.messages.baseUrlCopied')); | ||
| setTimeout(() => setIsCopied(false), 2000); | ||
| }; |
There was a problem hiding this comment.
The setTimeout call inside handleCopy can lead to a memory leak and a React warning ("Can't perform a React state update on an unmounted component") if the component unmounts before the timeout completes. A more robust approach is to manage this side effect using a useEffect hook with a cleanup function. This ensures the timer is cleared when the component unmounts.
Here's a suggested refactoring:
const [isCopied, setIsCopied] = useState(false);
useEffect(() => {
if (isCopied) {
const timerId = setTimeout(() => {
setIsCopied(false);
}, 2000);
return () => clearTimeout(timerId);
}
}, [isCopied]);
const handleCopy = async () => {
await navigator.clipboard.writeText(baseUrl);
toast.success(t('apikeys.messages.baseUrlCopied'));
setIsCopied(true);
};| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button size='icon' variant='ghost' className='shrink-0' onClick={handleCopy}> | ||
| {isCopied ? <Copy className='h-3.5 w-3.5 text-green-500' /> : <Link className='h-3.5 w-3.5' />} |
There was a problem hiding this comment.
For better UI consistency, consider using a CheckIcon to indicate a successful copy, similar to other copy buttons in the application (e.g., for code blocks). The Copy icon is typically associated with the action of copying, not the success state.
| {isCopied ? <Copy className='h-3.5 w-3.5 text-green-500' /> : <Link className='h-3.5 w-3.5' />} | |
| {isCopied ? <CheckIcon className='h-3.5 w-3.5 text-green-500' /> : <Link className='h-3.5 w-3.5' />} |
|
close #1072 |
1 participant
No description provided.