[dashboard] Add tooltip to project names #5323
Conversation
|
Fixes #5122 |
|
Looking at this now! 👀 |
There was a problem hiding this comment.
Looks good @laushinka! 🏀
Left one minor comment regarding UX. Feel free to open a follow up issue for this if you think it will significantly increase the scope of this PR. 🏓
Cross-posting #5122 (comment) for visibility. 👀
| @@ -91,7 +91,9 @@ export function WorkspaceEntry({ desc, model, isAdmin, stopWorkspace }: Props) { | |||
| </ItemFieldIcon> | |||
| <ItemField className="w-3/12 flex flex-col"> | |||
| <a href={startUrl.toString()}><div className="font-medium text-gray-800 dark:text-gray-200 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 dark:text-gray-500 hover:text-blue-600 dark:hover:text-blue-400">{project || 'Unknown'}</div></a> | |||
| <Tooltip content={project ? 'https://' + project : ''}> | |||
There was a problem hiding this comment.
issue: Could we enforce a max width for the tooltip component as the tooltip overflows the container width when the path is long?
There was a problem hiding this comment.
Haha I wrote my comment just when you wrote this one.
|
LGTM label has been added. Git tree hash: 80ffa61b199b45e6f24eb1a822819b4c4bbb1203
|
|
@gtsiolis At first I set a max width for it, but then I decided against it since it's a temporary solution and we wanted users to be able to see the whole link for now. Let me know what you think :) |
@laushinka Sounds mergeable! I'd only add a max width and allow text wrapping to avoid overflowing the body container. Feel free to pass this to someone for taking a closer look at the code, too. Skipping the max width could also suffice for now. Let's see what other think. |
|
note: Because of #5323 (comment), you'll also need an approval from someone from the engineering team. Cc @laushinka |
No worries, definitely no strong feelings against adding the max-width and wrapping it. Will make the changes now :) |
laushinka
force-pushed
the
laushinka/dashboard-long-project-5122
branch
from
ab050e5
to
ecb0f92
Compare
|
/werft run 👍 started the job as gitpod-build-laushinka-dashboard-long-project-5122.4 |
laushinka
force-pushed
the
laushinka/dashboard-long-project-5122
branch
from
ecb0f92
to
24c0384
Compare
|
Thanks! Could you squash the changes? |
laushinka
force-pushed
the
laushinka/dashboard-long-project-5122
branch
from
24c0384
to
cb59e10
Compare
|
/lgtm |
|
LGTM label has been added. Git tree hash: b51b65f5e959cae631b685631373f576eac837c3
|
laushinka
force-pushed
the
laushinka/dashboard-long-project-5122
branch
from
cb59e10
to
3a79668
Compare
|
/lgtm |
|
LGTM label has been added. Git tree hash: 2caa4efb0e156bc3cd335b6883318951455ce7ee
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gtsiolis, JanKoehnlein Associated issue: #5122 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes #5122