Skip to content

[usage] Align credits and time in usage view #13173

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

Merged
merged 1 commit into from
Sep 22, 2022

Conversation

easyCZ
Copy link
Member

@easyCZ easyCZ commented Sep 21, 2022

Description

Right-aligns credits and time in the usage view

Related Issue(s)

How to test

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide

@easyCZ easyCZ requested a review from a team September 21, 2022 12:44
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Sep 21, 2022
@@ -310,7 +310,7 @@ function UsageView({ attributionId }: UsageViewProps) {
</div>
<div className="my-auto" />
<div className="flex flex-col col-span-3 my-auto">
<span className="text-gray-400 dark:text-gray-500 truncate font-medium">
<span className="text-gray-400 text-right dark:text-gray-500 truncate font-medium">
Copy link
Member

Choose a reason for hiding this comment

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

did you also want to remove the truncate (elipses)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was planning to do that later as I need to run for a meeting but if it's that simple, can do :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done now

Copy link
Member

Choose a reason for hiding this comment

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

the original issue complained about truncation on the time not the credits.
Screenshot 2022-09-21 at 10 12 27

Copy link
Member Author

Choose a reason for hiding this comment

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

Those are 2 different PRs, and 2 different Issues:
This one is #13154 which deals with alignment and truncation. There's another PR which deals with Credit rounding - #13056

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I'm talking about the truncation of the time property. See screenshot ☝️

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@svenefftinge svenefftinge Sep 21, 2022

Choose a reason for hiding this comment

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

you removed the truncate for the 'usage' not the 'time'.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm sorry, I don't follow what you mean. I'm removing truncation from the number of minutes that a workspace ran (as per the issue), not the usage record timestamp.

In the preview env, it renders as below, which should be the expected outcome
Screenshot 2022-09-21 at 17 13 04

@easyCZ easyCZ force-pushed the mp/usage-align-credits-time branch from 2025087 to 93a15b9 Compare September 21, 2022 12:50
Copy link
Member

@svenefftinge svenefftinge left a comment

Choose a reason for hiding this comment

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

Looks good, adding hold in case you want to include the removal of 'truncate' on the time property

/hold

@easyCZ easyCZ force-pushed the mp/usage-align-credits-time branch 2 times, most recently from fd5f176 to 82efb17 Compare September 21, 2022 14:25
@easyCZ easyCZ force-pushed the mp/usage-align-credits-time branch from 82efb17 to f5b1d81 Compare September 21, 2022 14:45
@easyCZ
Copy link
Member Author

easyCZ commented Sep 21, 2022

/werft run with-preview

👍 started the job as gitpod-build-mp-usage-align-credits-time.5
(with .werft/ from main)

@easyCZ
Copy link
Member Author

easyCZ commented Sep 22, 2022

I'm unholding this PR as the current form looks to solve the problem as required. Happy to do a follow-up.

/unhold

@roboquat roboquat merged commit f58284e into main Sep 22, 2022
@roboquat roboquat deleted the mp/usage-align-credits-time branch September 22, 2022 08:46
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/XS team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants