Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/grumpy-islands-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@channel.io/bezier-react": patch
---

Enhance the interface of `Radio` and `RadioGroup` components. Change `Radio` to render children when their value is evaluated as true.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* External dependencies */
import React, { forwardRef, useEffect } from 'react'
import React, { forwardRef, useEffect, isValidElement } from 'react'

/* Internal dependencies */
import { StackItem } from 'Components/Stack'
Expand Down Expand Up @@ -48,7 +48,7 @@ forwardedRef: React.Ref<HTMLDivElement>,
role={role}
>
{ React.Children.map(children, (child, index) => (
<StackItem key={(child as React.ReactElement)?.key ?? index}>
<StackItem key={isValidElement(child) ? child.key : index}>
{ child }
</StackItem>
)) }
Expand Down
6 changes: 6 additions & 0 deletions packages/bezier-react/src/components/Forms/Radio/Radio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import RadioProps from './Radio.types'
export const RADIO_TEST_ID = 'bezier-react-radio'
export const RADIO_HANDLE_TEST_ID = 'bezier-react-radio-handle'

/**
* @deprecated Use `RadioGroup` instead.
*/
function Radio(
{
as,
Expand Down Expand Up @@ -85,4 +88,7 @@ function Radio(
)
}

/**
* @deprecated Use `RadioGroup` instead.
*/
export default forwardRef(Radio)
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ interface RadioOptions {
onClick?: (value: any, e: MouseEvent) => void
}

/**
* @deprecated Use `RadioGroup` instead.
*/
export default interface RadioProps extends
BezierComponentProps,
ChildrenProps,
Expand Down
28 changes: 17 additions & 11 deletions packages/bezier-react/src/components/Forms/RadioGroup/Radio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,11 @@ import useId from 'Hooks/useId'
import { RadioProps } from './RadioGroup.types'
import * as Styled from './RadioGroup.styled'

/**
* `Radio` is a checkable button, known as a radio button.
* It is must be a child of `RadioGroup`.
*/
export const Radio = forwardRef(function Radio({
function RadioImpl<Value extends string>({
children,
id: idProp,
...rest
}: RadioProps, forwardedRef: React.Ref<HTMLButtonElement>) {
}: RadioProps<Value>, forwardedRef: React.Ref<HTMLButtonElement>) {
const id = useId(idProp, 'bezier-radio')

return (
Expand All @@ -23,10 +19,20 @@ export const Radio = forwardRef(function Radio({
id={id}
{...rest}
>
{ /* @ts-ignore FIXME(@ed): Delete after applying polymorphic props */ }
<Styled.Label htmlFor={id}>
{ children }
</Styled.Label>
{ children && (
/* @ts-ignore FIXME(@ed): Delete after applying polymorphic props */
<Styled.Label htmlFor={id}>
{ children }
</Styled.Label>
) }
</Styled.RadioGroupPrimitiveItem>
)
})
}

/**
* `Radio` is a checkable button, known as a radio button.
* It should be a child of `RadioGroup`.
*/
export const Radio = forwardRef(RadioImpl) as <Value extends string>(
props: RadioProps<Value> & { ref?: React.ForwardedRef<HTMLButtonElement> }
) => ReturnType<typeof RadioImpl<Value>>
Original file line number Diff line number Diff line change
@@ -1,33 +1,18 @@
/* External dependencies */
import React, { forwardRef } from 'react'
import React, { forwardRef, isValidElement } from 'react'
import * as RadioGroupPrimitive from '@radix-ui/react-radio-group'

/* Internal dependencies */
import { Stack, StackItem } from 'Components/Stack'
import useFormFieldProps from 'Components/Forms/useFormFieldProps'
import { RadioGroupProps } from './RadioGroup.types'

/**
* `RadioGroup` is a set of checkable buttons, known as radio buttons.
*
* `RadioGroup` is a context of the `Radio` components. also, it renders an element which has the 'radiogroup' role.
* It controls all the parts of a radio group.
*
* @example
*
* ```tsx
* // Anatomy of the RadioGroup
* <RadioGroup>
* <Radio />
* </RadioGroup>
* ```
*/
export const RadioGroup = forwardRef(function RadioGroup({
function RadioGroupImpl<Value extends string>({
children,
spacing = 0,
direction = 'vertical',
...rest
}: RadioGroupProps, forwardedRef: React.Ref<HTMLDivElement>) {
}: RadioGroupProps<Value>, forwardedRef: React.Ref<HTMLDivElement>) {
const formFieldProps = useFormFieldProps(rest)

return (
Expand All @@ -43,11 +28,30 @@ export const RadioGroup = forwardRef(function RadioGroup({
direction={direction}
>
{ React.Children.map(children, (child, index) => (
<StackItem key={child?.key ?? index}>
<StackItem key={isValidElement(child) ? child.key : index}>
{ child }
</StackItem>
)) }
</Stack>
</RadioGroupPrimitive.Root>
)
})
}

/**
* `RadioGroup` is a set of checkable buttons, known as radio buttons.
*
* `RadioGroup` is a context of the `Radio` components. also, it renders an element which has the 'radiogroup' role.
* It controls all the parts of a radio group.
*
* @example
*
* ```tsx
* // Anatomy of the RadioGroup
* <RadioGroup>
* <Radio />
* </RadioGroup>
* ```
*/
export const RadioGroup = forwardRef(RadioGroupImpl) as <Value extends string>(
Copy link
Contributor

@yangwooseong yangwooseong Feb 13, 2023

Choose a reason for hiding this comment

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

여기서 generic 이 제대로 동작안해서 assertion 이 필요한 이유가 궁금해서 찾아봤더니 이런 이유가 있었네요..! (#)
#1126 이것도 assertion 을 주거나 여기 나온 다른 방법을 써야할 듯 하군요

We only make higher order function type inferences when the source and target types are both pure function types, i.e. types with a single call signature and no other members.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 공유 감사합니다. 저도 꼼꼼히 확인해봐야할듯.

props: RadioGroupProps<Value> & { ref?: React.ForwardedRef<HTMLDivElement> }
) => ReturnType<typeof RadioGroupImpl<Value>>
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ import { BezierComponentProps, ChildrenProps } from 'Types/ComponentProps'
import { FormComponentProps } from 'Components/Forms'
import { StackProps } from 'Components/Stack'

interface RadioGroupOptions {
interface RadioGroupOptions<Value extends string = string> {
/**
* The controlled value of the radio item to check.
* Should be used in conjunction with `onValueChange`.
*/
value?: string
value?: Value
/**
* The value of the radio item that should be checked when initially rendered.
* Use when you do not need to control the state of the radio items.
*/
defaultValue?: string
defaultValue?: Value
/**
* The name of the group.
* Submitted with its owning form as part of a name/value pair.
Expand All @@ -35,14 +35,14 @@ interface RadioGroupOptions {
/**
* Event handler called when the value changes.
*/
onValueChange?: (value: string) => void
onValueChange?: (value: Value) => void
}

interface RadioOptions {
interface RadioOptions<Value extends string = string> {
/**
* The value given as data when submitted with a `RadioGroupProps.name`.
*/
value: string
value: Value
/**
* The unique id of the radio item. It is created automatically by default.
* It used by the label element in the radio item.
Expand All @@ -52,16 +52,16 @@ interface RadioOptions {

type RadioFormComponentProps = Pick<FormComponentProps, 'disabled' | 'required'>

export interface RadioGroupProps extends
export interface RadioGroupProps<Value extends string = string> extends
BezierComponentProps,
ChildrenProps<React.ReactElement[] | React.ReactElement>,
ChildrenProps,
RadioFormComponentProps,
Omit<React.HTMLAttributes<HTMLDivElement>, keyof RadioGroupOptions | keyof RadioGroupPrimitive.RadioGroupProps>,
RadioGroupOptions {}
Omit<React.HTMLAttributes<HTMLDivElement>, keyof RadioGroupOptions<Value> | keyof RadioGroupPrimitive.RadioGroupProps>,
RadioGroupOptions<Value> {}

export interface RadioProps extends
export interface RadioProps<Value extends string = string> extends
BezierComponentProps,
ChildrenProps,
RadioFormComponentProps,
Omit<React.HTMLAttributes<HTMLButtonElement>, keyof RadioOptions>,
RadioOptions {}
Omit<React.HTMLAttributes<HTMLButtonElement>, keyof RadioOptions<Value>>,
RadioOptions<Value> {}