Skip to content
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

[dashboard] Fix workspace entry truncation #4416

Closed
wants to merge 2 commits into from

Conversation

corneliusludmann
Copy link
Contributor

The actual change is the commit 9f41b1d that fixes the truncation in the workspace list:

Before This PR
image image

Besides this, this PR also has the commit 4476666 that adds tooltips for the values. The rationale behind this is that it should be possible to read the whole non-truncated value somehow:

image

The downside is that the tooltip is always there, no matter if truncated or not. I don't know an easy way to show the tooltip for truncated values only. If we don't like this behavior, we can drop this commit.

@corneliusludmann corneliusludmann requested a review from gtsiolis June 7, 2021 19:29
@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 8, 2021

/werft run

👍 started the job as gitpod-build-clu-fix-workspace-entry-truncate.4

@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 9, 2021

/werft run

👍 started the job as gitpod-build-clu-fix-workspace-entry-truncate.5

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for truncating these @corneliusludmann! 🍐

<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}>
Copy link
Contributor

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).

Context Changes
Screenshot 2021-06-10 at 2 22 19 AM Screenshot 2021-06-10 at 2 22 10 AM

Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @corneliusludmann! 🏓

</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>
Copy link
Contributor

@gtsiolis gtsiolis Jun 9, 2021

Choose a reason for hiding this comment

The 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants