Skip to content
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

[ui] Add a chip to indicate pro features #3358

Merged
merged 22 commits into from
Apr 10, 2024

Conversation

bharatkashyap
Copy link
Member

@bharatkashyap bharatkashyap commented Apr 5, 2024

Screen.Recording.2024-04-09.at.5.03.59.PM.mov
  • Also makes fixes to the <UpgradeAlert /> to change the nature of the alerts to info instead of error, which was jarring

Screenshot 2024-04-05 at 1 06 17 PM

  • Also some minor fixes on the typography of the <PageOptionsPanel />':
    Screenshot 2024-04-05 at 1 09 15 PM

  • Also some updates to documentation copy to better reflect the paid nature of the gated features

- "only available for free for a limited time"
+ "is a paid feature"

@bharatkashyap bharatkashyap added the design: ui Design label Apr 5, 2024
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 5, 2024

  • I think we should remove the blue icon, this represents MUI X Pro plan, which is not the same as Toolpad Commercial plan.
  • I would look into how Metabase does paid feature discovery https://www.metabase.com/pricing/. It's not clear to me that this should be visible in the product itself (and not only in the docs)

@bharatkashyap bharatkashyap changed the title [ui] Add a badge to indicate pro features [ui] Add a chip to indicate pro features Apr 5, 2024
Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

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

I'm not convinced this will scale as the amount pro features grows, but that will be a good problem to have. Until then I'm ok with this solution 👍

{!isPaidPlan ? (
<Tooltip
title={
<Typography variant="inherit">
Copy link
Member

@Janpot Janpot Apr 9, 2024

Choose a reason for hiding this comment

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

Just a string should be fine, no?

Suggested change
<Typography variant="inherit">

<Chip
variant="outlined"
color="primary"
component={'a'}
Copy link
Member

Choose a reason for hiding this comment

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

I wish prettier would fix these

Suggested change
component={'a'}
component="a"

verticalAlign: 'inherit',
mx: '0.2rem',
'& :hover': {
cursor: 'pointer',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cursor: 'pointer',

use the clickable prop of the Chip

type?: AlertColor;
feature?: string;
warning?: string;
variant?: 'alert' | 'chip';
Copy link
Member

Choose a reason for hiding this comment

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

Just export two components, an alert and a chip

@bharatkashyap bharatkashyap merged commit b2fa891 into mui:master Apr 10, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Authorization paid plan error message
3 participants