-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[dashboard] Improve explicit usage-based billing attribution UX #12151
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
Conversation
/werft run 👍 started the job as gitpod-build-jx-billing-account.1 |
I believe this is roughly there, but the UX seems a bit sub-optimal:
Maybe this is a good time to also do a quick UI polish? E.g. by implementing #11498 here. @gtsiolis when no choice is available (e.g. you have zero teams with usage-based enabled), there is no available choice. I've added a small placeholder text to provide users with a basic hint. Do you think this is a good idea? Or maybe you have alternative proposals? 👀 |
3c7d035
to
43cbad1
Compare
43cbad1
to
9a5362c
Compare
🪙 ➡️ 🎰 /werft run 👍 started the job as gitpod-build-jx-billing-account.4 |
9a5362c
to
51c0cd6
Compare
Looking at this now! 👀 |
@jankeromnes How can I enable UBP for personal account? I only see Plans and Team Plans in user settings. |
Thanks @gtsiolis!
This has changed recently. In order to enable UBP for you personal account, you (currently) need to be part of a team that has UBP enabled and a paid subscription. This will likely change again when we introduce billing for individual users, but for now this is how to enable it and see the attribution UI. |
@jankeromnes I have also enabled UBP for a team (feel free to join the team in the preview env if helpful) but I still can not see the billing options in user settings, see screenshot below.
Two more comments regarding UX:
|
@gtsiolis Followed your invite link, can you make me owner? 🙏 @jankeromnes What version is this branch based on? Older than yesterday? |
@geropl DONE |
@gtsiolis This was unrelated to this PR and triggered by me misconfiguring ConfigCat. 😬 @jankeromnes You were right: For this reason we rely on the FF being enabled for users as well 🙈 so both |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works, code LGTM!
/hold For @gtsiolis feedback. ✨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, @jankeromnes! ✨
Left one minor issue comment about layout scalability and two nitpicks which are out of the scope of these changes. 🏀
@@ -62,27 +82,43 @@ export function BillingAccountSelector(props: { onSelected?: () => void }) { | |||
<div> | |||
<p>Bill all my usage to:</p> | |||
<div className="mt-4 flex space-x-3"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue(non-blocking): Could we wrap the cards and limit the number of entries per row as seen in #11498 (comment) to avoid breaking the layout or overflowing the page container?
Multiple billing accounts |
---|
![]() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! ✨
Thanks, @jankeromnes! 🏀
Hey, many thanks for the quick reviews and excellent feedback! Will implement all recommendations. 💻 |
51c0cd6
to
a5b1a59
Compare
Releasing the breaks 😎 🚀 /unhold |
Description
Related Issue(s)
Fixes #12097
Fixes #11498
How to test
/billing
Release Notes
Documentation
Werft options: