Skip to content

Conversation

@mayan-000
Copy link
Collaborator

@mayan-000 mayan-000 commented Nov 28, 2025

This pull request introduces three new UI components—Label, RadioGroup, Skeleton, and Separator—to the frappe-ui-react package, each built on top of Radix UI primitives. It also adds their respective type definitions and comprehensive Storybook stories for documentation and testing. Additionally, the required Radix UI dependencies are added to the package manifest. These changes enhance the design system by providing configurable, accessible, and themable form and layout primitives.

New Component Additions

  • Added a configurable Label component (src/components/label/index.tsx) supporting size, weight, variant, disabled, required, and optional props, with Radix UI integration.
  • Added a flexible RadioGroup component (src/components/radiogroup/index.tsx) with support for sizes, themes, variants, orientation, option descriptions, and disabled states, built on Radix UI.
  • Added a customizable Separator component (src/components/separator/index.tsx) supporting size, color, variant, and orientation, leveraging Radix UI.

Type Definitions

  • Defined prop types for Label, RadioGroup, Skeleton, and Separator components in their respective types.ts files for type safety and IntelliSense.

Storybook Documentation

  • Added Storybook stories for Label and RadioGroup components to demonstrate usage, variants, and states, aiding in visual documentation and testing.

Dependency Management

  • Updated package.json to include new Radix UI dependencies required for the added components: @radix-ui/react-label, @radix-ui/react-radio-group, and @radix-ui/react-separator.

@mayan-000 mayan-000 requested a review from mohdsayed November 28, 2025 07:24
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no label component on the figma file or the docs where is this referenced from?

cc: @b1ink0

"@popperjs/core": "^2.11.8",
"@radix-ui/react-dialog": "^1.1.14",
"@radix-ui/react-dropdown-menu": "^2.1.15",
"@radix-ui/react-label": "^2.1.8",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Radix UI is used. These should be derived from Base UI

lg: "text-lg",
};

const themeClasses: Record<RadioGroupTheme, string> = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no notion of themes on the docs or design why is this here? Is it documented anywhere else.

name,
orientation = "vertical",
}) => {
const sizeClasses: Record<RadioGroupSize, string> = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Constants should not be declared in the components.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no radio group in the docs or figma. I'm assuming this is an extension of radio-button from the figma.

}
`}
>
<div className="font-medium">{option.label}</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

div used for text

>
<div className="font-medium">{option.label}</div>
{option.description && (
<div
Copy link
Collaborator

Choose a reason for hiding this comment

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

div used for text

size === "sm" ? "text-xs" : "text-sm"
} text-ink-gray-5 mt-0.5`}
>
{option.description}
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no description in the designs

className={`
${sizeClasses[size]}
${themeClasses[theme]}
rounded-full border-2
Copy link
Collaborator

Choose a reason for hiding this comment

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

default state should have no border.

${themeClasses[theme]}
rounded-full border-2
focus:outline-none focus-visible:ring-2 focus-visible:ring-outline-gray-3
disabled:cursor-not-allowed disabled:opacity-50
Copy link
Collaborator

Choose a reason for hiding this comment

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

no-opacity changes in designs

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.

3 participants