Skip to content

Fix remaining issues for simplified team plans #10182

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.

Sign up for GitHub

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 5 commits into from
May 30, 2022
Merged

Fix remaining issues for simplified team plans #10182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3718579
[dashboard] Un-hide the new Team Billing menu entry
jankeromnes May 23, 2022
4de3308
[payment] Fix race condition on multiple new Team Plan joins
jankeromnes May 27, 2022
2c1f29a
[dashboard] Rename DropDown 'contextMenuWidth' → 'customClasses' to r…
jankeromnes May 30, 2022
36f5485
[dashboard] Support optionally rendering DropDowns with link colors
jankeromnes May 30, 2022
f27b0e0
[dashboard] Improve design of new Team Billing
jankeromnes May 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/dashboard/src/Menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ export default function Menu() {
)}
<div className="flex h-full rounded-lg hover:bg-gray-100 dark:hover:bg-gray-800">
<ContextMenu
classes="w-64 left-0"
customClasses="w-64 left-0"
menuEntries={[
{
title: userFullName,
Expand Down
4 changes: 2 additions & 2 deletions components/dashboard/src/admin/License.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default function License() {
subtitle="License associated with your Gitpod installation"
>
<div className="flex flex-row space-x-4">
<Card>
<Card className="w-72 h-64">
<span>
{licenseLevel}
{paid}
Expand All @@ -70,7 +70,7 @@ export default function License() {
</div>
</span>
</Card>
<SolidCard>
<SolidCard className="w-72 h-64">
<span>
<div className="my-2">{statusMessage}</div>
<p className="dark:text-gray-500 font-semibold">Registered Users</p>
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/admin/TeamDetail.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export default function TeamDetail(props: { team: Team }) {
<ItemField className="flex items-center my-auto">
<span className="text-gray-400 capitalize">
<DropDown
contextMenuWidth="w-32"
customClasses="w-32"
activeEntry={m.role}
entries={[
{
Expand Down
8 changes: 6 additions & 2 deletions components/dashboard/src/components/Arrow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@
* See License-AGPL.txt in the project root for license information.
*/

function Arrow(props: { up: boolean }) {
function Arrow(props: { up: boolean; customBorderClasses?: string }) {
return (
<span
className="mx-2 border-gray-400 dark:border-gray-600 group-hover:border-gray-600 dark:group-hover:border-gray-400"
className={
"mx-2 " +
(props.customBorderClasses ||
"border-gray-400 dark:border-gray-600 group-hover:border-gray-600 dark:group-hover:border-gray-400")
}
style={{
marginTop: 2,
marginBottom: 2,
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/components/Card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function Card(p: { className?: string; onClick?: () => void; children?: React.Re
return (
<div
className={
"flex flex-col rounded-xl w-72 h-64 px-4 bg-gray-800 dark:bg-gray-100 text-gray-200 dark:text-gray-500 " +
"flex flex-col rounded-xl px-4 bg-gray-800 dark:bg-gray-100 text-gray-200 dark:text-gray-500 " +
(p.className || "")
}
onClick={p.onClick}
Expand Down
4 changes: 2 additions & 2 deletions components/dashboard/src/components/ContextMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { Link } from "react-router-dom";
export interface ContextMenuProps {
children?: React.ReactChild[] | React.ReactChild;
menuEntries: ContextMenuEntry[];
classes?: string;
customClasses?: string;
}

export interface ContextMenuEntry {
Expand Down Expand Up @@ -98,7 +98,7 @@ function ContextMenu(props: ContextMenuProps) {
{expanded ? (
<div
className={`mt-2 z-50 bg-white dark:bg-gray-900 absolute flex flex-col border border-gray-200 dark:border-gray-800 rounded-lg truncated ${
props.classes || "w-48 right-0"
props.customClasses || "w-48 right-0"
}`}
data-analytics='{"button_type":"context_menu"}'
>
Expand Down
19 changes: 13 additions & 6 deletions components/dashboard/src/components/DropDown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import ContextMenu from "./ContextMenu";

export interface DropDownProps {
prefix?: string;
contextMenuWidth?: string;
customClasses?: string;
renderAsLink?: boolean;
activeEntry?: string;
entries: DropDownEntry[];
}
Expand All @@ -35,14 +36,20 @@ function DropDown(props: DropDownProps) {
},
};
});
const font =
"text-gray-400 dark:text-gray-500 text-sm leading-1 group hover:text-gray-600 dark:hover:text-gray-400 transition ease-in-out";
const defaultFont = "text-gray-400 dark:text-gray-500 hover:text-gray-600 dark:hover:text-gray-400 ";
const asLinkFont = "text-blue-500 dark:text-blue-400 hover:text-blue-600 dark:hover:text-blue-500";
const asLinkArrowBorder =
"border-blue-500 dark:border-blue-400 group-hover:border-blue-600 dark:group-hover:border-blue-500";
return (
<ContextMenu menuEntries={enhancedEntries} classes={`${props.contextMenuWidth} right-0`}>
<span className={`py-2 cursor-pointer ${font}`}>
<ContextMenu menuEntries={enhancedEntries} customClasses={`${props.customClasses} right-0`}>
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@gtsiolis gtsiolis May 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@jankeromnes jankeromnes May 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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.:

Screenshot 2022-05-30 at 16 01 13 Left Screenshot 2022-05-30 at 16 01 18 Right Screenshot 2022-05-30 at 16 01 31 Left
Screenshot 2022-05-30 at 16 01 56
Right 		Screenshot 2022-05-30 at 16 02 41 Right Screenshot 2022-05-30 at 16 40 43 Center

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, thanks for clarifying, @jankeromnes!

<span
className={`py-2 cursor-pointer text-sm leading-1 group ${
props.renderAsLink ? asLinkFont : defaultFont
} transition ease-in-out`}
>
{props.prefix}
{current}
<Arrow up={false} />
<Arrow up={false} customBorderClasses={props.renderAsLink ? asLinkArrowBorder : undefined} />
</span>
</ContextMenu>
);
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/components/PendingChangesDropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export default function PendingChangesDropdown(props: { workspaceInstance?: Work
return <div className="text-sm text-gray-400 dark:text-gray-500">No Changes</div>;
}
return (
<ContextMenu menuEntries={menuEntries} classes="w-64 max-h-48 overflow-scroll mx-auto left-0 right-0">
<ContextMenu menuEntries={menuEntries} customClasses="w-64 max-h-48 overflow-scroll mx-auto left-0 right-0">
<p className="flex justify-center text-gitpod-red">
<span>
{totalChanges} Change{totalChanges === 1 ? "" : "s"}
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/components/PillLabel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
export default function PillLabel(props: { children?: React.ReactNode; type?: "info" | "warn"; className?: string }) {
const infoStyle = "bg-blue-50 text-blue-500 dark:bg-blue-500 dark:text-blue-100";
const warnStyle = "bg-orange-100 text-orange-700 dark:bg-orange-600 dark:text-orange-100";
const style = `ml-2 px-3 py-1 text-sm uppercase rounded-xl ${props.type === "warn" ? warnStyle : infoStyle} ${
const style = `px-3 py-1 text-sm uppercase rounded-xl ${props.type === "warn" ? warnStyle : infoStyle} ${
props.className
}`;
return <span className={style}>{props.children}</span>;
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/components/SolidCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function SolidCard(p: { className?: string; onClick?: () => void; children?: Rea
return (
<div
className={
"flex flex-col rounded-xl w-72 h-64 px-4 bg-gray-100 dark:bg-gray-800 text-gray-600 dark:text-gray-600 " +
"flex flex-col rounded-xl px-4 bg-gray-100 dark:bg-gray-800 text-gray-600 dark:text-gray-400 " +
(p.className || "")
}
onClick={p.onClick}
Expand Down
5 changes: 4 additions & 1 deletion components/dashboard/src/projects/NewProject.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,10 @@ export default function NewProject() {
</p>
<div className={`mt-2 flex-col ${noReposAvailable && isGitHub() ? "w-96" : ""}`}>
<div className="px-8 flex flex-col space-y-2" data-analytics='{"label":"Identity"}'>
<ContextMenu classes="w-full left-0 cursor-pointer" menuEntries={getDropDownEntries(accounts)}>
<ContextMenu
customClasses="w-full left-0 cursor-pointer"
menuEntries={getDropDownEntries(accounts)}
>
<div className="w-full">
{!selectedAccount && user && user.name && user.avatarUrl && (
<>
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/projects/Prebuilds.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export default function (props: { project?: Project; isAdminDashboard?: boolean
</div>
<div className="flex-1" />
<div className="py-3 pl-3">
<DropDown prefix="Prebuild Status: " contextMenuWidth="w-32" entries={statusFilterEntries()} />
<DropDown prefix="Prebuild Status: " customClasses="w-32" entries={statusFilterEntries()} />
</div>
{!isLoadingPrebuilds && prebuilds.length === 0 && !props.isAdminDashboard && (
<button onClick={() => triggerPrebuild(null)} className="ml-2">
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/projects/ProjectSettings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export default function () {
title={
<span>
Enable Incremental Prebuilds{" "}
<PillLabel type="warn" className="font-semibold mt-2 py-0.5 px-2 self-center">
<PillLabel type="warn" className="font-semibold mt-2 ml-2 py-0.5 px-2 self-center">
Beta
</PillLabel>
</span>
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/settings/Preferences.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export default function Preferences() {

<h3 className="mt-12">
Dotfiles{" "}
<PillLabel type="warn" className="font-semibold mt-2 py-0.5 px-2 self-center">
<PillLabel type="warn" className="font-semibold mt-2 ml-2 py-0.5 px-2 self-center">
Beta
</PillLabel>
</h3>
Expand Down
4 changes: 2 additions & 2 deletions components/dashboard/src/teams/Members.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export default function () {
<div className="py-3 pl-3">
<DropDown
prefix="Role: "
contextMenuWidth="w-32"
customClasses="w-32"
activeEntry={roleFilter === "owner" ? "Owner" : roleFilter === "member" ? "Member" : "All"}
entries={[
{
Expand Down Expand Up @@ -226,7 +226,7 @@ export default function () {
m.role
) : (
<DropDown
contextMenuWidth="w-32"
customClasses="w-32"
activeEntry={m.role}
entries={[
{
Expand Down
75 changes: 46 additions & 29 deletions components/dashboard/src/teams/TeamBilling.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,14 @@ export default function TeamBilling() {
title="Billing"
subtitle="Manage team billing and plans."
>
<h3>{!teamPlan ? "No billing plan" : "Plan"}</h3>
<h3>{!teamPlan ? "Upgrade Team Plan" : "Team Plan"}</h3>
<h2 className="text-gray-500">
{!teamPlan ? (
<div className="flex space-x-1">
<span>Select a new billing plan for this team. Currency:</span>
<span>Upgrade team plan to access unlimited workspace hours, and more. Currency:</span>
<DropDown
contextMenuWidth="w-32"
customClasses="w-32"
renderAsLink={true}
activeEntry={currency}
entries={[
{
Expand All @@ -172,12 +173,12 @@ export default function TeamBilling() {
<div className="mt-4 space-x-4 flex">
{isLoading && (
<>
<SolidCard>
<SolidCard className="w-72 h-64">
<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">
<div className="w-full h-full flex flex-col items-center justify-center">
<Spinner className="h-5 w-5 animate-spin" />
</div>
Expand All @@ -188,14 +189,22 @@ export default function TeamBilling() {
<>
{availableTeamPlans.map((tp) => (
<>
<SolidCard
className="cursor-pointer hover:bg-gray-200 dark:hover:bg-gray-700"
onClick={() => checkout(tp)}
>
<SolidCard className="w-72 h-72">
<div className="px-2 py-5 flex-grow flex flex-col">
<div className="font-medium text-base">{tp.name}</div>
<div className="font-semibold text-gray-500 text-sm">Unlimited hours</div>
<div className="mt-8 font-semibold text-sm">Includes:</div>
<div className="font-semibold text-gray-800 dark:text-gray-100 text-lg">
{tp.name}
</div>
<div className="font-semibold text-gray-400 dark:text-gray-600 text-sm">
Unlimited hours
</div>
<div className="mt-2">
<PillLabel type="warn" className="font-semibold normal-case text-sm">
{members.length} x {Currency.getSymbol(tp.currency)}
{tp.pricePerMonth} = {Currency.getSymbol(tp.currency)}
{members.length * tp.pricePerMonth} per month
</PillLabel>
</div>
<div className="mt-4 font-semibold text-sm">Includes:</div>
<div className="flex flex-col items-start text-sm">
{(featuresByPlanType[tp.type] || []).map((f) => (
<span className="inline-flex space-x-1">
Expand All @@ -204,12 +213,10 @@ export default function TeamBilling() {
</span>
))}
</div>
<div className="flex-grow flex flex-col items-end justify-end">
<PillLabel type="warn" className="font-semibold normal-case text-sm">
{members.length} x {Currency.getSymbol(tp.currency)}
{tp.pricePerMonth} = {Currency.getSymbol(tp.currency)}
{members.length * tp.pricePerMonth} per month
</PillLabel>
<div className="flex-grow flex flex-col items-stretch justify-end">
<button className="m-0" onClick={() => checkout(tp)}>
Upgrade to {tp.name}
</button>
</div>
</div>
</SolidCard>
Expand All @@ -219,10 +226,14 @@ export default function TeamBilling() {
)}
{!isLoading && teamPlan && (
<>
<Card>
<Card className="w-72 h-64">
<div className="px-2 py-5 flex-grow flex flex-col">
<div className="font-bold text-base">{teamPlan.name}</div>
<div className="font-semibold text-gray-500 text-sm">Unlimited hours</div>
<div className="font-semibold text-gray-100 dark:text-gray-800 text-lg">
{teamPlan.name}
</div>
<div className="font-semibold text-gray-400 dark:text-gray-600 text-sm">
Unlimited hours
</div>
<div className="mt-8 font-semibold text-sm">Includes:</div>
<div className="flex flex-col items-start text-sm">
{(featuresByPlanType[teamPlan.type] || []).map((f) => (
Expand All @@ -232,25 +243,31 @@ export default function TeamBilling() {
</span>
))}
</div>
<div className="flex-grow flex flex-col items-end justify-end"></div>
<div className="flex-grow flex flex-col items-stretch justify-end"></div>
</div>
</Card>
{!teamSubscription ? (
<SolidCard>
<SolidCard className="w-72 h-64">
<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">
Copy link
Contributor

Choose a reason for hiding this comment

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

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!

Copy link
Contributor Author

@jankeromnes jankeromnes May 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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)

Copy link
Contributor

Choose a reason for hiding this comment

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

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. 🛹

Copy link
Contributor Author

@jankeromnes jankeromnes May 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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). 💭

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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")

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

<div className="px-2 py-5 flex-grow flex flex-col">
<div className="font-medium text-base text-gray-400">Members</div>
<div className="font-semibold text-base text-gray-600">{members.length}</div>
<div className="mt-8 font-medium text-base text-gray-400">Next invoice on</div>
<div className="font-semibold text-base text-gray-600">
<div className="font-medium text-base text-gray-400 dark:text-gray-600">
Members
</div>
<div className="font-semibold text-base text-gray-600 dark:text-gray-400">
{members.length}
</div>
<div className="mt-4 font-medium text-base text-gray-400 dark:text-gray-600">
Next invoice on
</div>
<div className="font-semibold text-base text-gray-600 dark:text-gray-400">
{guessNextInvoiceDate(teamSubscription.startDate).toDateString()}
</div>
<div className="flex-grow flex flex-col items-end justify-end">
<div className="flex-grow flex flex-col items-stretch justify-end">
<button
onClick={() => {
if (team) {
Expand Down
8 changes: 4 additions & 4 deletions components/dashboard/src/teams/TeamSettings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ export function getTeamSettingsMenu(params: { team?: Team; showPaymentUI?: boole
},
...(showPaymentUI
? [
// {
// title: "Billing",
// link: [`/t/${team?.slug}/billing`],
// },
{
title: "Billing",
link: [`/t/${team?.slug}/billing`],
},
]
: []),
];
Expand Down
Loading