-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[dashboard] Fix workspace entry truncation #4416
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,19 +86,31 @@ export function WorkspaceEntry({ desc, model, isAdmin, stopWorkspace }: Props) { | |
<WorkspaceStatusIndicator instance={desc?.latestInstance} /> | ||
</div> | ||
<div className="flex flex-col w-3/12"> | ||
<a href={startUrl.toString()}><div className="font-medium text-gray-800 dark:text-gray-100 truncate hover:text-blue-600 dark:hover:text-blue-400">{ws.id}</div></a> | ||
<a href={project ? 'https://' + project : undefined}><div className="text-sm overflow-ellipsis truncate text-gray-400 hover:text-blue-600 dark:hover:text-blue-400">{project || 'Unknown'}</div></a> | ||
<Tooltip content={ws.id}> | ||
<a href={startUrl.toString()}> | ||
<div className="font-medium text-gray-800 dark:text-gray-100 overflow-ellipsis truncate w-full hover:text-blue-600 dark:hover:text-blue-400">{ws.id}</div> | ||
</a> | ||
</Tooltip> | ||
<Tooltip content={project || 'Unknown'}> | ||
<a href={project ? 'https://' + project : undefined}> | ||
<div className="text-sm overflow-ellipsis truncate w-full text-gray-400 hover:text-blue-600 dark:hover:text-blue-400">{project || 'Unknown'}</div> | ||
</a> | ||
</Tooltip> | ||
</div> | ||
<div className="flex w-4/12 truncate overflow-ellipsis"> | ||
<div className="flex flex-col"> | ||
<div className="text-gray-500 overflow-ellipsis truncate">{ws.description}</div> | ||
<div className="flex flex-col w-4/12"> | ||
<Tooltip content={ws.description}> | ||
<div className="text-gray-500 overflow-ellipsis truncate w-full">{ws.description}</div> | ||
</Tooltip> | ||
<Tooltip content={ws.contextURL}> | ||
<a href={ws.contextURL}> | ||
<div className="text-sm text-gray-400 overflow-ellipsis truncate hover:text-blue-600 dark:hover:text-blue-400">{ws.contextURL}</div> | ||
<div className="text-sm text-gray-400 overflow-ellipsis truncate w-full hover:text-blue-600 dark:hover:text-blue-400">{ws.contextURL}</div> | ||
</a> | ||
</div> | ||
</Tooltip> | ||
</div> | ||
<div className="flex flex-col items-start w-2/12"> | ||
<div className="text-gray-500 truncate">{currentBranch}</div> | ||
<Tooltip content={currentBranch} className="w-full"> | ||
<div className="text-gray-500 overflow-ellipsis truncate w-full">{currentBranch}</div> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: May I suggest to leave the truncation on and remove most, if not all, tooltips for this iteration? What do you think? 🎈 Ideally, we could make the Context column so that it contains more compact information (see #3594). This way, we would only need a tooltip on the first row of the context column. Additionally, on the Pending Changes column we could truncate the branch name and offer a copy-to-clipboard button without also having a tooltip. |
||
</Tooltip> | ||
<PendingChangesDropdown workspaceInstance={desc.latestInstance} /> | ||
</div> | ||
<div className="flex w-2/12 self-center"> | ||
|
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.
issue: Adding the tooltip on all element can a) make the interface feel slightly overwelmed, b) feel redandunt on some elements like the workspace name, and introduce some visual imbalance (see screenshots).
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.
Yes, absolutely. Since the truncation is fixed in #4454 already, I'll close this one.
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.
Thanks @corneliusludmann! 🏓