- Notifications
You must be signed in to change notification settings - Fork928
feat: add premium license banner for custom roles page#14595
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes from5 commits
036012668d160835dc1cea8f6d9cdc21678b20e7bfeffe086fe52caef42ac7e2485442a407fb9File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2,28 +2,39 @@ import type { Interpolation, Theme } from "@emotion/react"; | ||
| import TaskAltIcon from "@mui/icons-material/TaskAlt"; | ||
| import Button from "@mui/material/Button"; | ||
| import Link from "@mui/material/Link"; | ||
| import { EnterpriseBadge, PremiumBadge } from "components/Badges/Badges"; | ||
| import { Stack } from "components/Stack/Stack"; | ||
| import type { FC, ReactNode } from "react"; | ||
| import theme from "theme"; | ||
| import { docs } from "utils/docs"; | ||
| export interface PaywallProps { | ||
| type: "enterprise" | "premium"; | ||
| message: string; | ||
| description?: ReactNode; | ||
| documentationLink?: string; | ||
| } | ||
| export const Paywall: FC<PaywallProps> = ({ | ||
| type, | ||
| message, | ||
| description, | ||
| documentationLink, | ||
| }) => { | ||
| return ( | ||
| <div | ||
| css={[ | ||
| styles.root, | ||
| (theme) => ({ | ||
| backgroundImage: `linear-gradient(160deg, transparent, ${theme.branding.paywall[type].background})`, | ||
| border: `1px solid ${theme.branding.paywall[type].border}`, | ||
| }), | ||
| ]} | ||
| > | ||
| <div> | ||
| <Stack direction="row" alignItems="center" css={{ marginBottom: 24 }}> | ||
| <h5 css={styles.title}>{message}</h5> | ||
| {type === "enterprise" ?<EnterpriseBadge /> : <PremiumBadge />} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. My main worry with this setup is that because the conditional "flip" for displaying the text is getting repeated so much, there's a higher risk of someone making a typo and accidentally mixing text and bullet points between paywall types I know it isn't DRY, but just to keep things firmly grouped together, my preference would be to have two variants of the component (that can be kept un-exported as private implementation details) – one for premium and one for enterprise There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Basically, I don't think there's a great way to fully DRY up the code, so we have to choose between DRYing up the content or the markup/styling. But because this potentially involves a lot of money, I'd always choose content and making sure customers can't ever get incorrect information. A single missed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. This was meant to be a temporary solution as I originally thought the enterprise license would only remain for a short time longer. But now I am going to update this to completely remove the enterprise license and leave only the premium | ||
| </Stack> | ||
| {description && <p css={styles.description}>{description}</p>} | ||
| @@ -36,20 +47,32 @@ export const Paywall: FC<PaywallProps> = ({ | ||
| Read the documentation | ||
| </Link> | ||
| </div> | ||
| <div css={styles.separator} /> | ||
| <Stack direction="column" alignItems="center" spacing={3}> | ||
| <ul css={styles.featureList}> | ||
| <li css={styles.feature}> | ||
| <FeatureIcon type={type} /> | ||
| {type === "premium" | ||
| ? "High availability & workspace proxies" | ||
| : "Template access control"} | ||
| </li> | ||
| <li css={styles.feature}> | ||
| <FeatureIcon type={type} /> | ||
| {type === "premium" | ||
| ? "Multi-org & role-based access control" | ||
| : "User groups"} | ||
| </li> | ||
| <li css={styles.feature}> | ||
| <FeatureIcon type={type} /> | ||
| {type === "premium" | ||
| ? "24x7 global support with SLA" | ||
| : "24 hour support"} | ||
| </li> | ||
| <li css={styles.feature}> | ||
| <FeatureIcon type={type} /> | ||
| {type === "premium" | ||
| ? "Unlimited Git & external auth integrations" | ||
| : "Audit logs"} | ||
| </li> | ||
| </ul> | ||
| <Button | ||
| @@ -60,15 +83,27 @@ export const Paywall: FC<PaywallProps> = ({ | ||
| variant="outlined" | ||
| color="neutral" | ||
| > | ||
| Learn about{type === "enterprise" ? "Enterprise" : "Premium"} | ||
| </Button> | ||
| </Stack> | ||
| </div> | ||
| ); | ||
| }; | ||
| export interface FeatureIconProps { | ||
| type: "enterprise" | "premium"; | ||
| } | ||
| const FeatureIcon: FC<FeatureIconProps> = ({ type }) => { | ||
| return ( | ||
| <TaskAltIcon | ||
| css={[ | ||
| (theme) => ({ | ||
| color: theme.branding.paywall[type].border, | ||
| }), | ||
| ]} | ||
| /> | ||
| ); | ||
| }; | ||
| const styles = { | ||
| @@ -77,12 +112,9 @@ const styles = { | ||
| flexDirection: "row", | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| minHeight: 280, | ||
| marginBottom: 32, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Ireally disagree with having any kind of external margin value on a general-purpose component Again, maybe not DRY, but since the external spacing is strictly something the parent component would need to be worried about, I'd move that logic here. The margin isn't inherently tied to the content of the component, and now we can't use it for other purposes without having a hard-coded margin value be enforced There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. got it, removed the marginBottom | ||
| padding: 24, | ||
| borderRadius: 8, | ||
| gap: 32, | ||
| }), | ||
| @@ -93,12 +125,9 @@ const styles = { | ||
| margin: 0, | ||
| }, | ||
| description: (theme) => ({ | ||
| fontFamily: "inherit", | ||
| maxWidth: 460, | ||
| fontSize: 14, | ||
| }), | ||
| separator: (theme) => ({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Making a note for myself: we should update our color theme so that it works better on colored backgrounds. The gray divider color looks really washed out on a saturated colored background Member
| ||
| width: 1, | ||
| @@ -110,7 +139,7 @@ const styles = { | ||
| listStyle: "none", | ||
| margin: 0, | ||
| marginRight: 8, | ||
| padding: "024px", | ||
| fontSize: 14, | ||
| fontWeight: 500, | ||
| }, | ||
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.