Skip to content

[dashboard] Fix Prebuilds item truncate #10472

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

Closed

Conversation

flaming-codes
Copy link

Description

This PR fixes a small truncation issue w/ text in the Prebuilds items.

Related Issue(s)

Fixes #9103

How to test

No specific tests for this change are available.

Release Notes

NONE

Documentation

Screenshots

Description Image
Before fix Screenshot 2022-06-04 at 20 37 17
After fix Screenshot 2022-06-04 at 20 37 08

@flaming-codes flaming-codes requested a review from a team June 4, 2022 18:47
@flaming-codes flaming-codes changed the title [dashboard] Prebuilds item truncate [dashboard] Fix Prebuilds item truncate Jun 4, 2022
@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 6, 2022

/werft run

👍 started the job as gitpod-build-fix-prebuilds-item-truncate-fork.0
(with .werft/ from main)

@gtsiolis gtsiolis added the do-not-merge/cla-pending CLA has not been signed label Jun 6, 2022
@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 6, 2022

/werft run

👍 started the job as gitpod-build-fix-prebuilds-item-truncate-fork.1
(with .werft/ from main)

@stale
Copy link

stale bot commented Jun 18, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Jun 18, 2022
@stale stale bot closed this Jun 24, 2022
@flaming-codes
Copy link
Author

@gtsiolis I signed the CLA, so everything should have been fine to merge those changes. Is there anything on my side to do, so that contributions are accepted? Thanks!

@gtsiolis gtsiolis reopened this Jun 27, 2022
@stale stale bot removed the meta: stale This issue/PR is stale and will be closed soon label Jun 27, 2022
@stale
Copy link

stale bot commented Jul 10, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Jul 10, 2022
@flaming-codes
Copy link
Author

@gtsiolis What's the status on this from your side? I signed the CLA a long time ago, is there anything to do on my part? Thanks!

@stale stale bot removed the meta: stale This issue/PR is stale and will be closed soon label Jul 11, 2022
@gtsiolis
Copy link
Contributor

Thanks for reaching out, @flaming-codes! Let me link the comment from #10268 (comment).

@meysholdt meysholdt added cla: accepted ✔️ and removed do-not-merge/cla-pending CLA has not been signed labels Jul 26, 2022
@jankeromnes
Copy link
Contributor

jankeromnes commented Aug 1, 2022

/werft run

👍 started the job as gitpod-build-fix-prebuilds-item-truncate-fork.2
(with .werft/ from main)

@jankeromnes
Copy link
Contributor

jankeromnes commented Aug 1, 2022

/werft run with-preview

👍 started the job as gitpod-build-fix-prebuilds-item-truncate-fork.3
(with .werft/ from main)

@jankeromnes
Copy link
Contributor

jankeromnes commented Aug 1, 2022

/werft run with-preview

👍 started the job as gitpod-build-fix-prebuilds-item-truncate-fork.4
(with .werft/ from main)

Copy link
Contributor

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Many thanks for improving this!

Since the preview doesn't want to come up, I tried testing this on the live Gitpod app using my browser devtools.

I believe this improves the situation a bit, but in general, I'm somewhat unhappy with the general state of our table/column layout (in the Prebuilds list and the Workspaces list). We seem to be using truncate and overflow-hidden somewhat at random (sometimes multiple times on elements and their containers), yet some columns still don't have a consistent width in some layouts.

Ideally, in the future, we could clean this up by:

  • Using a consistent width for all "column" top-level containers in an element, combined with overflow-hidden (this strictly prevents columns from growing larger than they should -- e.g. long branchnames currently overflow)
  • Using truncate on every text sub-element that might have overflowing text (so that the hard cut of the text is not too "brutal" but has an ellipsis ...)

Still, no point in delaying this improvement of the status quo because there are also other possible improvements to be made. 🙂 Thus, approving this PR.

Many thanks!

@jankeromnes
Copy link
Contributor

jankeromnes commented Aug 1, 2022

Trying to bypass the failing build using a build without a preview:

/werft run

👍 started the job as gitpod-build-fix-prebuilds-item-truncate-fork.5
(with .werft/ from main)

@geropl
Copy link
Member

geropl commented Aug 1, 2022

/hold to unblock merge pool

@jankeromnes
Copy link
Contributor

Thanks! Should pass auto-merge now:

/unhold

AlexTugarev and others added 12 commits August 1, 2022 19:53
so that we can gradually roll this out in prod
The billing client tries to connect to the usage component gRPC server
before the gRPC server is ready. This is a non-fatal error as gRPC
dialing has built-in backoff/retry logic but it does produce ugly
warnings in the usage component logs.

Here we use a simple solution to delay the self-connection attempt for a
short period to give the gRPC server time to start.
@flaming-codes
Copy link
Author

flaming-codes commented Aug 1, 2022

Hi team,

there were a lot of merge conflicts when trying to update my PR with the latest changes - no wonder, as my initial fix is now almost 2 months old, as of writing.

I think I neither have the time nor energy to spend to fix PR-conflicts for such small changes, therefore I propose to delete this PR and apply the initial fix w/ a new one.

Thanks for all your work and Gitpod as a product, but these super long delays in my PR reviews that result in only fixing conflicts isn't the experience I yearn for.

All the best,
Tom

@flaming-codes flaming-codes deleted the fix-prebuilds-item-truncate branch August 1, 2022 19:59
@jankeromnes
Copy link
Contributor

Hi @flaming-codes,

Sorry that it took so long to review your changes that so many conflicts appeared... We're still not very good at reviewing community contributions in a consistent and timely manner, and we have a lot to learn & to improve there.

I'd totally understand if you don't have the time or energy to re-do the fix in a new PR. As an alternative, I believe I could salvage your commit into a new PR, and work towards merging that one. Would you be okay with this? I'd cc you on the new PRs so that you can follow along if interested, and since you've already signed the CLA, there would be no further actions needed on your part.

Thanks again,
Jan

@flaming-codes
Copy link
Author

Hi Jan,

As an alternative, I believe I could salvage your commit into a new PR, and work towards merging that one. Would you be okay with this?

Sure, works fine for me!

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.

Fix table alignment in project prebuilds list