Skip to content

[dashboard] display credit usage #15295

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 2 commits into from
Dec 12, 2022

Conversation

svenefftinge
Copy link
Member

@svenefftinge svenefftinge commented Dec 12, 2022

Description

Show the amount of credits used per workspace class
Screenshot 2022-12-12 at 10 14 50

Related Issue(s)

Fixes #15275

How to test

Release Notes

NONE

Documentation

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

@svenefftinge svenefftinge requested a review from a team December 12, 2022 08:29
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-sefftinge-prebuilds-workspace-starts-14726.1 because the annotations in the pull request description changed
(with .werft/ from main)

@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Dec 12, 2022
@@ -215,7 +215,7 @@ func (c *Client) GetCustomerByAttributionID(ctx context.Context, attributionID s
}

if len(customers) == 0 {
return nil, status.Errorf(codes.NotFound, "no team customer found for attribution_id: %s", attributionID)
Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated drive-by change

@@ -2155,7 +2155,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl {
try {
customer = (await this.billingService.getStripeCustomer({ attributionId })).customer;
} catch (e) {
console.error(e);
Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated drive-by change

@svenefftinge
Copy link
Member Author

/hold

@svenefftinge svenefftinge force-pushed the sefftinge/prebuilds-workspace-starts-14726 branch from 7263f4c to e5f7c59 Compare December 12, 2022 09:15
<span className="text-gray-900 dark:text-gray-100">{balance}</span>
<span className="text-gray-900 dark:text-gray-100">
{balance.toLocaleString(undefined, { maximumFractionDigits: 2 })}
</span>
Copy link
Member Author

@svenefftinge svenefftinge Dec 12, 2022

Choose a reason for hiding this comment

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

drive-by, fixes #15285

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Today I learned about Number.toLocaleString(). 💡

@svenefftinge
Copy link
Member Author

/unhold

@@ -48,7 +48,7 @@ export default function SelectWorkspaceClass(props: SelectWorkspaceClassProps) {
{supportedClasses.map((c) => {
return (
<WorkspaceClass
additionalStyles="w-80 h-32"
additionalStyles="w-80"
Copy link
Contributor

@jankeromnes jankeromnes Dec 12, 2022

Choose a reason for hiding this comment

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

Unfortunately, removing the fixed height causes the bottom padding to look enormous 😞 but I didn't have a good suggestion for how to get it right without significant changes. So, maybe let's iterate on the design of the workspace class selector in follow-ups.

Copy link
Member Author

Choose a reason for hiding this comment

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

by enormous you mean more than what is shown in screenshot in the initial comment?

Copy link
Contributor

@jankeromnes jankeromnes Dec 12, 2022

Choose a reason for hiding this comment

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

I mean the empty vertical space between the dot and the bottom border of the card:

Before

Screenshot 2022-12-12 at 15 09 52

After

Screenshot 2022-12-12 at 15 10 10

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, ok that is intentional. cc @gtsiolis

Copy link
Contributor

@gtsiolis gtsiolis Dec 12, 2022

Choose a reason for hiding this comment

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

Padding changes look (acceptable). ✔️

I'd separate the consumption rate (10 credits/hour) from the resources (dot) indicator (● ●) information to scale better, but can be done along with more spacing improvements later when we introduce more than two workspace classes.

Thanks for the ping, @svenefftinge! 🏓
Thanks for caring about the spacing, @jankeromnes! ✨

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.

Thanks for this clarification, and for the drive-by fixes! 🎉

Looks good to me (modulo the new enormous bottom margin mentioned in-line).

@roboquat roboquat merged commit 5c1407f into main Dec 12, 2022
@roboquat roboquat deleted the sefftinge/prebuilds-workspace-starts-14726 branch December 12, 2022 14:09
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Dec 13, 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/S team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[workspace classes] Display credit burn rate
4 participants