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

feat(ui): toggle component and more #1913

Merged
merged 6 commits into from
Nov 19, 2024
Merged

feat(ui): toggle component and more #1913

merged 6 commits into from
Nov 19, 2024

Conversation

VanishMax
Copy link
Contributor

@VanishMax VanishMax commented Nov 18, 2024

Changeset:

  • Add Toggle UI component – adapt it from ui-deprecated
  • Add medium density – new density type introduced in the DEX Figma, implemented in at least Tabs component
  • Add xxs Text style – new typography type with 11px font-size
  • Improve the styles of Tabs component – add support for medium density

Check here: https://penumbra-ui-preview--pr1913-fix-tab-styles-tf62cbwe.web.app/

@VanishMax VanishMax requested a review from a team November 18, 2024 13:18
@VanishMax VanishMax self-assigned this Nov 18, 2024
Copy link

changeset-bot bot commented Nov 18, 2024

🦋 Changeset detected

Latest commit: a4e36a3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@penumbra-zone/ui Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Nov 18, 2024

Visit the preview URL for this PR (updated for commit a4e36a3):

https://penumbra-ui-preview--pr1913-fix-tab-styles-tf62cbwe.web.app

(expires Tue, 26 Nov 2024 10:17:07 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 709d729610ef7a6369b23f1cb2b820a60cc685b1

Comment on lines +24 to +26
export type DensityProps = DensityPropType & {
children?: ReactNode;
} & (SelectedDensity extends 'sparse'
? { sparse: true; compact?: never }
: { compact: true; sparse?: never });
};
Copy link
Contributor

Choose a reason for hiding this comment

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

question: can't we do this?

export type Density = 'sparse' | 'medium' | 'compact';

export type DensityProps = {
  density: Density;
  children?: ReactNode;
};

Copy link
Contributor

Choose a reason for hiding this comment

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

though if we specifically need those names as a field, we could also do this:

export type DensityPropType =
  | { sparse: true; medium?: never; compact?: never }
  | { medium: true; sparse?: never; compact?: never }
  | { compact: true; sparse?: never; medium?: never };

export type DensityProps = DensityPropType & {
  children?: ReactNode;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Density and Text components initially had this style of props that is more neat: <Density compact/>, <Text body/>. I personally don't mind the union type but it is already a bit too late to change – so many components depend on a shorter syntax.

Changed the types to the version from your second comment

@VanishMax VanishMax merged commit bf2e541 into main Nov 19, 2024
8 checks passed
@VanishMax VanishMax deleted the fix/tab-styles branch November 19, 2024 10:25
@VanishMax VanishMax restored the fix/tab-styles branch November 19, 2024 10:57
@VanishMax VanishMax deleted the fix/tab-styles branch November 19, 2024 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants