- Notifications
You must be signed in to change notification settings - Fork1k
refactor: replace task prompt by workspace name in the topbar#19531
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Things look really good overall! Just had some ideas for tweaking a few things
| </TooltipProvider> | ||
| <h1className="m-0 text-base font-medium truncate">{task.prompt}</h1> | ||
| <h1className="m-0 ml-2 text-base font-medium truncate"> |
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.
I think that technically it won't be a huge deal, but could we swap theml-2 forpl-2? That would reduce the risk of CSS styling side effects
| </RouterLink> | ||
| </Button> | ||
| <divclassName="ml-auto gap-2 flex items-center"> | ||
| <TooltipProviderdelayDuration={0}> |
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.
Do we want the duration to be set so aggressively low? Radix's default value is 700ms
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.
Updated it to be 250.
| return( | ||
| <Button | ||
| disabled={showCopiedSuccess} | ||
| onClick={copyToClipboard} | ||
| size="sm" | ||
| variant="subtle" | ||
| className="p-0 min-w-0" | ||
| > | ||
| {showCopiedSuccess ?( | ||
| <> | ||
| <CheckIcon/> Copied! | ||
| </> | ||
| ) :( | ||
| <> | ||
| <CopyIcon/> Copy | ||
| </> | ||
| )} | ||
| </Button> | ||
| ); |
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.
In isolation, I think this is perfectly fine, but do we want to centralize the implementation for this, so we can make sure the UI stays more consistent?
I'm seeing a few other buttons in the codebase that are wired up very similarly (making a call touseClipboard, using theCheckIcon andCopyIcon components), and I'm just worried about them drifting over time
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.
I agree with you, but I would create a separate issue to investigate that and make it more consistent.
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.
a1546b5 intomainUh oh!
There was an error while loading.Please reload this page.
Fixes#19524
Screenshot:
Demo:
Screen.Recording.2025-08-25.at.14.59.15.mov
Changes: