Fix remaining issues for simplified team plans #10182

jankeromnes · 2022-05-23T07:25:02Z

Description

Improve and ship the simplified Team Billing. ✨

TODO:

Un-hide the new Team Billing menu entry
Polish the UI
Fix race condition on multiple team joins 🐞

Related Issue(s)

Fixes #10057

How to test

Create a Team
Navigate to Team Settings > Team Billing -- it should look nice ✨
Upgrade your Team to either Professional or Unleashed
- (provide bogus billing details, for example type "a" into every required field, and use the proposed fake credit card)
Invite two more members, who should join at almost exactly the same time (within ~30s)
Check Chargebee-test for new charges. There should be:
- one "Pro-rated upgrade from 1 to 2"
- one "Pro-rated upgrade from 2 to 3"

(If the bug isn't fixed, you'd see one "from 1 to 2" invoice, and another "from 1 to 3" invoice(!) that's double the amount)

Screenshot of a successful test:

Release Notes

Fix, polish, and release new simplified Team Billing

Documentation

/werft with-payment=true

jankeromnes · 2022-05-23T09:22:29Z

The UI ~~currently~~ looks like this:

No Billing	Active Billing

A few issues when comparing with the Figma spec:

The currency switcher should be blue
Dark theme font color should be fixed
~~Reword price label?~~

@gtsiolis do your keen eyes spot anything else? 🐞 🔍 👀

gtsiolis · 2022-05-23T20:09:05Z

Looking at this now! 👀

gtsiolis

Thanks, @jankeromnes! Overall looks great. ✨

I couldn't find any code changes in this PR related to the UX comments left below, which made it harder to gather all these comments and find a place to add them. I assume these visual changes were part of another pull request, right?

Left some first round review comments. Let me know what you think.

components/dashboard/src/teams/TeamBilling.tsx

jankeromnes · 2022-05-24T03:49:43Z

Wow, many thanks @gtsiolis for the in-depth UX review and spot-on suggestions! 🪄

I couldn't find any code changes in this PR related to the UX comments left below, which made it harder to gather all these comments and find a place to add them. I assume these visual changes were part of another pull request, right?

Ah, whoops, indeed. Most of these rough visual changes were actually implemented in #8041, but were left hidden from users by removing the new menu entry. 🕶️

Thank you for still managing to do the in-depth review! 🙏

jankeromnes · 2022-05-30T09:54:07Z

Another UI pass done ✨ Current state of the UI:

No billing	Team Unleashed

Remaining open questions:

~~Hide the word "Team" in plan names?~~
Replace card hover/click by a big primary button?

This reverts commit 3479ced.

…eflect reality

gtsiolis

UX LGTM, @jankeromnes! Left one last round of non-blocking comments.

components/dashboard/src/teams/TeamBilling.tsx

components/dashboard/src/components/DropDown.tsx

gtsiolis · 2022-05-30T12:36:07Z

components/dashboard/src/components/DropDown.tsx

    return (
-        <ContextMenu menuEntries={enhancedEntries} classes={`${props.contextMenuWidth} right-0`}>
-            <span className={`py-2 cursor-pointer ${font}`}>
+        <ContextMenu menuEntries={enhancedEntries} customClasses={`${props.customClasses} right-0`}>


thought: Wondering if we need the custom classes altogether here. 💭

I believe we do, because the context-menu sometimes need to be left-aligned or right-aligned, among other things.

It's better to only use left-aligned items inside the dropdown menus. 💡

Do you have a handy instance of a right-alignment you're referring to?

I've only seen the dropdown in the workspace start page when using a desktop editor that's using center alignment and this should also change, see #6910.

If this is about the alignment of the component itself with the rest of the elements around it, we could probably move the alignment control outside this component, right?

It's better to only use left-aligned items inside the dropdown menus. 💡

Ah, sorry, I meant how the collapsible elements horizontally align with respect to the control, e.g.:

Left Right Left

Right Right Center

Ah, thanks for clarifying, @jankeromnes!

gtsiolis · 2022-05-30T12:38:23Z

components/dashboard/src/teams/TeamBilling.tsx

                                <div className="w-full h-full flex flex-col items-center justify-center">
                                    <Spinner className="h-5 w-5 animate-spin" />
                                </div>
                            </SolidCard>
                        ) : (
-                            <SolidCard>
+                            <SolidCard className="w-72 h-64">


thought: Hoping in the future we can drop these by introducing component variants that include card size, etc. instead of hard-coding sizes like this. We'll get there!

There are ways to achieve this with a single component, e.g. by using:

min-height on the card (to allow it to grow arbitrary tall depending on its content, but not below some minimum height)

items-stretch on the card container (to make all the cards the same height, even if one card has a lot of content but the others don't)

Yes, but let's keep this as is for now until the card component matures enough from design perspective to set a foundation of card sizes, size steps, etc.

Long-term, to keep the component flexible I'd love to introduce a stepper property to increase the width or height without using Tailwind's classes. 🧡

But no need to do this in the scope of this PR. 🛹

Also, it's good to keep in mind that Tailwind is so nice because it allows directly "hard-coding" style in a nice way (as opposed to hiding style under multiple levels of abstractions, making them really hard to adjust afterwards). 💭

Long-term, to keep the component flexible I'd love to introduce a stepper property to increase the width or height without using Tailwind's classes. 🧡

But, even better would be if the Cards always look perfectly-sized, depending on their contents, without needing a stepper or Tailwind classes, right? 😇 ("Default correct" is better than "lots of knobs and switches")

Also, it's good to keep in mind that Tailwind is so nice because it allows directly "hard-coding" style in a nice way

I'm also in favor of duplication over abstraction as it comes with multiple benefits, but in the long run it makes sense to abstract some component aspects to help a) avoid running into code reviews nitpicking dimensions or other attributes, b) avoid requesting UX reviews when possible to avoid, c) feeling more confident about the product UX, and more.

However, this issue will become more relevant as these components mature and are widely used. For now, as long as we have two component instances of the solid card component, any approach would suffice and does not worth keeping this PR from being merged. 🤝

But, even better would be if the Cards always look perfectly-sized, depending on their contents, without needing a stepper or Tailwind classes, right?

Debatable! 🙂

In most cases, the card component would need to use a specified width or height, which could be challenging to maintain when having multiple cards side-by-side that from a design perspective have to use fixed and specific dimensions regardless if some contents are missing. For example, see editor preferences, etc.

However, there's still need for having flexible card layouts without fixed width that fill the parent container on smaller viewports, etc.

components/dashboard/src/components/DropDown.tsx

jankeromnes · 2022-05-30T12:52:13Z

/hold

gtsiolis · 2022-05-30T12:55:02Z

/hold

@jankeromnes Also, regarding the hold comment in #10182 (comment), in case it helps, this still needs a review from @gitpod-io/engineering-webapp to be merged.

At least 1 approving review is required to merge this pull request.

jankeromnes · 2022-05-30T13:03:20Z

At least 1 approving review is required to merge this pull request.

Oh. Sounds like a bug if it's true -- you should absolutely be able to approve at least dashboard fixes & improvements. 🚢

Co-authored-by: George Tsiolis <[email protected]>

jankeromnes · 2022-05-30T13:48:06Z

/unhold

geropl · 2022-05-30T14:28:40Z

Starting review now...

geropl

@jankeromnes Tested and fixed: Thx for that, and approve! ✔️

I think I still managed to break it:

I think the reasons is the "Quantity" dropdown:

during initial upgrade I choose 10 (or sth)
I got billed 175 € - without gaining anything 😬
the subsequent 1st quantity upgrade (10 -> 2) did not trigger an addition charge being created (:+1: )
the 2nd quantity upgrade (2 -> 3) incurred the charge (+35€) for the current month (:+1: )

IMO we should remove the quantity dropdown if possible.

/hold feel free to address this in this PR or let's discuss/fix in a follow up! 👍

jankeromnes · 2022-05-30T15:55:32Z

@jankeromnes Tested and fixed: Thx for that, and approve! ✔️

Woohoo 🙌 many thanks!

during initial upgrade I choose 10 (or sth)

I got billed 175 € - without gaining anything 😬

"User buys too many seats on purpose, is then surprised to pay for too many seats" 😅

Joking aside, I agree that it would be best to hide this quantity selector that isn't needed. It's confusing at best, if not actively harmful.

Will see if there is an easy fix, but if there isn't, I'll just merge this as is and open a follow-up issue.

jankeromnes · 2022-05-30T16:15:22Z

I think the quantity selector can only be disabled in Chargebee's settings:

Chargebee plan settings	Disallow editing quantity

Happy to do this, but since it's a follow-up issue, let's unblock this PR first.

/unhold

EDIT: Filed this follow-up issue: https://github.com/gitpod-io/ops/issues/2543

roboquat added do-not-merge/work-in-progress do-not-merge/release-note-label-needed size/S release-note and removed do-not-merge/release-note-label-needed labels May 23, 2022

jankeromnes force-pushed the jx/fix-better-team-plans branch from cee2dda to 7c0d6d7 Compare May 23, 2022 07:42

roboquat added size/M and removed size/S labels May 23, 2022

gtsiolis self-requested a review May 23, 2022 09:34

gtsiolis reviewed May 23, 2022

View reviewed changes

jankeromnes force-pushed the jx/fix-better-team-plans branch from 7c0d6d7 to 27d0c83 Compare May 27, 2022 07:05

roboquat added size/L and removed size/M labels May 27, 2022

jankeromnes force-pushed the jx/fix-better-team-plans branch 3 times, most recently from b12a4ea to ef668ad Compare May 30, 2022 09:37

jankeromnes force-pushed the jx/fix-better-team-plans branch from ef668ad to aee8f75 Compare May 30, 2022 10:05

[dashboard] Un-hide the new Team Billing menu entry

3718579

This reverts commit 3479ced.

jankeromnes force-pushed the jx/fix-better-team-plans branch from aee8f75 to dab4a38 Compare May 30, 2022 12:00

jankeromnes marked this pull request as ready for review May 30, 2022 12:09

jankeromnes requested a review from a team May 30, 2022 12:09

roboquat removed the do-not-merge/work-in-progress label May 30, 2022

github-actions bot added the team: webapp Issue belongs to the WebApp team label May 30, 2022

This comment was marked as resolved.

Sign in to view

jankeromnes added 2 commits May 30, 2022 12:31

[payment] Fix race condition on multiple new Team Plan joins

4de3308

[dashboard] Rename DropDown 'contextMenuWidth' → 'customClasses' to r…

2c1f29a

…eflect reality

[dashboard] Support optionally rendering DropDowns with link colors

36f5485

jankeromnes force-pushed the jx/fix-better-team-plans branch from dab4a38 to 56f5b6b Compare May 30, 2022 12:31

gtsiolis approved these changes May 30, 2022

View reviewed changes

roboquat added the do-not-merge/hold label May 30, 2022

[dashboard] Improve design of new Team Billing

f27b0e0

Co-authored-by: George Tsiolis <[email protected]>

jankeromnes force-pushed the jx/fix-better-team-plans branch from 56f5b6b to f27b0e0 Compare May 30, 2022 13:47

roboquat removed the do-not-merge/hold label May 30, 2022

geropl self-assigned this May 30, 2022

geropl approved these changes May 30, 2022

View reviewed changes

roboquat added the do-not-merge/hold label May 30, 2022

roboquat removed the do-not-merge/hold label May 30, 2022

roboquat merged commit 39d66bc into main May 30, 2022

roboquat deleted the jx/fix-better-team-plans branch May 30, 2022 16:15

roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels May 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix remaining issues for simplified team plans #10182

Fix remaining issues for simplified team plans #10182

jankeromnes commented May 23, 2022 •

edited

Loading

jankeromnes commented May 23, 2022 •

edited

Loading

gtsiolis commented May 23, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment

jankeromnes commented May 24, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 30, 2022 •

edited

Loading

This comment was marked as resolved.

gtsiolis left a comment

gtsiolis May 30, 2022

jankeromnes May 30, 2022

gtsiolis May 30, 2022 •

edited

Loading

jankeromnes May 30, 2022 •

edited

Loading

gtsiolis May 30, 2022

gtsiolis May 30, 2022

jankeromnes May 30, 2022 •

edited

Loading

gtsiolis May 30, 2022

jankeromnes May 30, 2022 •

edited

Loading

jankeromnes May 30, 2022

gtsiolis May 30, 2022

jankeromnes commented May 30, 2022

gtsiolis commented May 30, 2022

jankeromnes commented May 30, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 30, 2022

geropl commented May 30, 2022

geropl left a comment

jankeromnes commented May 30, 2022 •

edited

Loading

jankeromnes commented May 30, 2022 •

edited

Loading

Fix remaining issues for simplified team plans #10182

Fix remaining issues for simplified team plans #10182

Conversation

jankeromnes commented May 23, 2022 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

Documentation

jankeromnes commented May 23, 2022 • edited Loading

gtsiolis commented May 23, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis left a comment

Choose a reason for hiding this comment

jankeromnes commented May 24, 2022 • edited by werft-gitpod-dev-com bot Loading

jankeromnes commented May 30, 2022 • edited Loading

This comment was marked as resolved.

gtsiolis left a comment

Choose a reason for hiding this comment

gtsiolis May 30, 2022

Choose a reason for hiding this comment

jankeromnes May 30, 2022

Choose a reason for hiding this comment

gtsiolis May 30, 2022 • edited Loading

Choose a reason for hiding this comment

jankeromnes May 30, 2022 • edited Loading

Choose a reason for hiding this comment

gtsiolis May 30, 2022

Choose a reason for hiding this comment

gtsiolis May 30, 2022

Choose a reason for hiding this comment

jankeromnes May 30, 2022 • edited Loading

Choose a reason for hiding this comment

gtsiolis May 30, 2022

Choose a reason for hiding this comment

jankeromnes May 30, 2022 • edited Loading

Choose a reason for hiding this comment

jankeromnes May 30, 2022

Choose a reason for hiding this comment

gtsiolis May 30, 2022

Choose a reason for hiding this comment

jankeromnes commented May 30, 2022

gtsiolis commented May 30, 2022

jankeromnes commented May 30, 2022 • edited by werft-gitpod-dev-com bot Loading

jankeromnes commented May 30, 2022

geropl commented May 30, 2022

geropl left a comment

Choose a reason for hiding this comment

jankeromnes commented May 30, 2022 • edited Loading

jankeromnes commented May 30, 2022 • edited Loading

jankeromnes commented May 23, 2022 •

edited

Loading

jankeromnes commented May 23, 2022 •

edited

Loading

gtsiolis commented May 23, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 24, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 30, 2022 •

edited

Loading

gtsiolis May 30, 2022 •

edited

Loading

jankeromnes May 30, 2022 •

edited

Loading

jankeromnes May 30, 2022 •

edited

Loading

jankeromnes May 30, 2022 •

edited

Loading

jankeromnes commented May 30, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 30, 2022 •

edited

Loading

jankeromnes commented May 30, 2022 •

edited

Loading