Skip to content

Commit

Permalink
Change the as prop to allow all element types and button to allow `…
Browse files Browse the repository at this point in the history
…as` prop (#1899)

<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] I tested the changes in various browsers. (or didn't have to)
  - Windows: Chrome, Edge, (Optional) Firefox
  - macOS: Chrome, Edge, Safari, (Optional) Firefox

## Related Issue
<!-- Please link to issue if one exists -->

- #1106

## Summary
<!-- Please brief explanation of the changes made -->

Change the `as` prop to allow all element types and button to allow `as`
prop

## Details
<!-- Please elaborate description of the changes -->

- as 속성이 모든 엘리먼트 타입을 지원하도록 변경합니다. 단순 HTML element type 외에도, Link 컴포넌트를
as 속성에 주입하는 등의 케이스가 있습니다. 이를 지원하여 breaking change를 줄이기 위한 변경입니다.
- `Button` 또한 마찬가지로 Link, a 태그 등으로 사용되는 케이스들이 있습니다. as 속성을 지원하도록 개선했습니다.

### Breaking change? (Yes/No)
<!-- If Yes, please describe the impact and migration path for users -->

No
  • Loading branch information
sungik-choi authored Jan 11, 2024
1 parent 0f9ee21 commit 996fa9a
Show file tree
Hide file tree
Showing 8 changed files with 13 additions and 33 deletions.
2 changes: 1 addition & 1 deletion .changeset/tender-years-camp.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@

`Button`'s interface changes.

- `Button` no longer supports `as` and `interpolation` prop.
- `Button` no longer supports `interpolation` prop.
- The enum values of `ButtonSize` and `ButtonColorVariant` are changed to kebab case.
5 changes: 1 addition & 4 deletions packages/bezier-react/src/components/Box/Box.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,17 @@ import {

type Display = 'block' | 'inline' | 'inline-block'

type BoxElementType = 'div' | 'span' | 'section' | 'legend' | 'ul' | 'ol' | 'li'

interface BoxOwnProps {
/**
* Display type of the box.
*/
display?: Display
}

// TODO: Make the polymorphic property stricter
export interface BoxProps extends
AlphaBezierComponentProps,
React.HTMLAttributes<HTMLElement>,
PolymorphicProps<BoxElementType>,
PolymorphicProps,
ChildrenProps,
LayoutProps,
MarginProps,
Expand Down
7 changes: 5 additions & 2 deletions packages/bezier-react/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ function ButtonSideContent({
}

export const Button = forwardRef<HTMLButtonElement, ButtonProps>(function Button({
as = 'button',
className,
testId = BUTTON_TEST_ID,
type = 'button',
Expand All @@ -113,6 +114,8 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(function Button
onClick,
...rest
}, forwardedRef) {
const Comp = as as 'button'

const handleClick = useCallback<React.MouseEventHandler<HTMLButtonElement>>((event) => {
if (!disabled) {
onClick?.(event)
Expand All @@ -123,7 +126,7 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(function Button
])

return (
<button
<Comp
// eslint-disable-next-line react/button-has-type
type={type}
ref={forwardedRef}
Expand Down Expand Up @@ -171,6 +174,6 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(function Button
<Spinner size={getSpinnerSize(size)} />
</div>
) }
</button>
</Comp>
)
})
2 changes: 2 additions & 0 deletions packages/bezier-react/src/components/Button/Button.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { type BezierIcon } from '@channel.io/bezier-icons'
import {
type AlphaBezierComponentProps,
type DisableProps,
type PolymorphicProps,
type SideContentProps,
type SizeProps,
} from '~/src/types/ComponentProps'
Expand Down Expand Up @@ -92,6 +93,7 @@ interface ButtonOwnProps {

export interface ButtonProps extends
AlphaBezierComponentProps,
PolymorphicProps,
SizeProps<ButtonSize>,
DisableProps,
SideContentProps<SideContent, SideContent>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,6 @@ describe('Modal', () => {
const trigger = getByRole('button', { name: TRIGGER_TEXT })
await user.click(trigger)
const [closeButton, closeIconButton] = getAllByRole('button')
// FIXME: Auto focusing not working properly in test environment
await user.tab()
await user.tab()
expect(document.activeElement).toBe(closeButton)
await user.tab()
Expand Down
4 changes: 1 addition & 3 deletions packages/bezier-react/src/components/Stack/Stack.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ type BaseAlignment = 'start' | 'center' | 'end' | 'stretch'
type Align = BaseAlignment | 'baseline'
type Justify = BaseAlignment | 'between'

type StackElementType = 'div' | 'section' | 'ul' | 'ol'

interface StackOwnProps {
/**
* Display type of the stack.
Expand Down Expand Up @@ -51,7 +49,7 @@ interface StackOwnProps {
export interface StackProps extends
AlphaBezierComponentProps,
React.HTMLAttributes<HTMLElement>,
PolymorphicProps<StackElementType>,
PolymorphicProps,
ChildrenProps,
LayoutProps,
MarginProps,
Expand Down
20 changes: 1 addition & 19 deletions packages/bezier-react/src/components/Text/Text.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,6 @@ type Typography =
| '30'
| '36'

type TextElementType =
| 'h1'
| 'h2'
| 'h3'
| 'h4'
| 'h5'
| 'h6'
| 'p'
| 'span'
| 'label'
| 'small'
| 'em'
| 'i'
| 'b'
| 'strong'
| 'legend'
| 'div'

type TextAlign = 'left' | 'center' | 'right'

interface TextOwnProps {
Expand Down Expand Up @@ -75,7 +57,7 @@ interface TextOwnProps {
export interface TextProps extends
AlphaBezierComponentProps,
Omit<React.HTMLAttributes<HTMLElement>, keyof TextOwnProps>,
PolymorphicProps<TextElementType>,
PolymorphicProps,
ChildrenProps,
MarginProps,
TextOwnProps {}
4 changes: 2 additions & 2 deletions packages/bezier-react/src/types/ComponentProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ export interface AlphaBezierComponentProps extends
Omit<RenderConfigProps, 'as'>,
Omit<StylableComponentProps, 'interpolation'> {}

export interface PolymorphicProps<T extends React.ElementType> {
export interface PolymorphicProps {
/**
* Element type to render.
*/
as?: T
as?: React.ElementType
}

export interface MarginProps {
Expand Down

0 comments on commit 996fa9a

Please sign in to comment.