Skip to content

Commit

Permalink
Migrate FormHelperText, FormErrorMessage, and FormLabel compone…
Browse files Browse the repository at this point in the history
…nt with scss (channel-io#1893)

<!--
  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 -->

<!-- Fixes #0000 -->

- channel-io#1733 

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

- `FormHelperText`, `FormErrorMessage`, `FormLabel`컴포넌트를 scss로 마이그레이션 하고
`as`, `interpolation`속성을 인터페이스에서 제거합니다.

## Details
<!-- Please elaborate description of the changes -->
- 각각의 컴포넌트를 scss로 마이그레이션할 때, `FormControl.styled.ts`에 있던 스타일 코드를 각 컴포넌트의
.module.scss로 이동시켰습니다. 또한 context로서 Wrapper컴포넌트를 주던 것을 `labelPosition`만
주고 각 컴포넌트의 .module.scss에서 스타일링 하도록 변경했습니다. `FornControl.module.scss`에서
스타일링 코드를 작성하고 각 컴포넌트에서는 여기에 있는 className을 사용하게 할 수도 있었는데, 컴포넌트간의 의존성을
만드는 것이 좋지 않아보여서 이렇게 했습니다.

- Ref타입으로 deprecated된 HTMLParamElement을 쓰던 것을 적절한 타입으로 변경합니다.

- 컴포넌트의 레이아웃이 `labelPosition`에 영향을 받기 때문에 일부러 `MarginProps`는 인터페이스로 노출하지
않았습니다.

- 지금은 `Text`의 속성을 그대로 노출하고 있는데, 사실상 고정된 스타일(color, typo)를 사용하고 있기 때문에 이런
속성역시 제거해야되나 고민됩니다. 어떻게 생각하시는지 궁금합니다 @sungik-choi

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

- Yes, `FormHelperText`, `FormErrorMessage`, `FormLabel` no longer
supports `as` and `interpolation` props.

## References
<!-- Please list any other resources or points the reviewer should be
aware of -->

- https://developer.mozilla.org/en-US/docs/Web/API/HTMLParamElement
  • Loading branch information
yangwooseong authored and sungik-choi committed Jan 16, 2024
1 parent 95ce44b commit 0ec84a4
Show file tree
Hide file tree
Showing 19 changed files with 385 additions and 555 deletions.
7 changes: 7 additions & 0 deletions .changeset/funny-peaches-report.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@channel.io/bezier-react": major
---

**Breaking Changes: Property updates in `FormLabel`, `FormHelperText`, and `FormErrorMessage` component**

No longer support `as` and `interpolation` property. Replace any usage of `interpolation` property with appropriate `style` or `className` implementations.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
.FormLabelWrapper {
&:where(.position-top) {
margin-bottom: 4px;
padding: 0 2px;
}

&:where(.position-left) {
display: flex;
grid-column: 1 / 1;
grid-row: 1 / 1;
align-items: center;
align-self: start;

height: var(--b-form-control-left-label-wrapper-height);
}
}

.FormHelperTextWrapper {
margin-top: 4px;
padding: 0 2px;

&:where(.position-left) {
grid-column: 2;
grid-row: 2 / 2;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -9,34 +9,10 @@ const Box = styled.div<InterpolationProps>`
${({ interpolation }) => interpolation}
`

export const TopLabelWrapper = styled(Box)`
padding: 0 2px;
margin-bottom: 4px;
`

export const TopHelperTextWrapper = styled(Box)`
padding: 0 2px;
margin-top: 4px;
`

export const Grid = styled(Box)`
display: grid;
grid-template-rows: repeat(2, auto);
grid-template-columns: ${LEFT_LABEL_MIN_WIDTH}px 1fr;
grid-column-gap: 12px;
align-items: center;
`

export const LeftLabelWrapper = styled(Box)`
display: flex;
grid-row: 1 / 1;
grid-column: 1 / 1;
align-items: center;
align-self: start;
height: var(--b-form-control-left-label-wrapper-height);
`

export const LeftHelperTextWrapper = styled(TopHelperTextWrapper)`
grid-row: 2 / 2;
grid-column: 2;
`
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import React, {
useState,
} from 'react'

import classNames from 'classnames'

import useId from '~/src/hooks/useId'
import { splitByBezierComponentProps } from '~/src/utils/props'
import { px } from '~/src/utils/style'
Expand All @@ -26,6 +28,7 @@ import {
} from './FormControl.types'
import { FormControlContextProvider } from './FormControlContext'

import styles from './FormControl.module.scss'
import * as Styled from './FormControl.styled'

export const FORM_CONTROL_TEST_ID = 'bezier-react-form-control'
Expand Down Expand Up @@ -112,10 +115,8 @@ export const FormControl = forwardRef<HTMLElement, FormControlProps>(function Fo
const getLabelProps = useCallback<LabelPropsGetter>(ownProps => ({
id: labelId,
htmlFor: fieldId,
className: classNames(styles.FormLabelWrapper, styles[`position-${labelPosition}`]),
typo: labelPosition === 'top' ? '13' : '14',
Wrapper: labelPosition === 'top'
? Styled.TopLabelWrapper
: Styled.LeftLabelWrapper,
...ownProps,
}), [
fieldId,
Expand All @@ -139,9 +140,7 @@ export const FormControl = forwardRef<HTMLElement, FormControlProps>(function Fo
id: helperTextId,
visible: isNil(formCommonProps?.hasError) || !formCommonProps?.hasError,
ref: setHelperTextNode,
Wrapper: labelPosition === 'top'
? Styled.TopHelperTextWrapper
: Styled.LeftHelperTextWrapper,
className: classNames(styles.FormHelperTextWrapper, labelPosition === 'left' && styles['position-left']),
...ownProps,
}), [
helperTextId,
Expand All @@ -153,9 +152,7 @@ export const FormControl = forwardRef<HTMLElement, FormControlProps>(function Fo
id: errorMessageId,
visible: isNil(formCommonProps?.hasError) || formCommonProps?.hasError,
ref: setErrorMessageNode,
Wrapper: labelPosition === 'top'
? Styled.TopHelperTextWrapper
: Styled.LeftHelperTextWrapper,
className: classNames(styles.FormHelperTextWrapper, labelPosition === 'left' && styles['position-left']),
...ownProps,
}), [
errorMessageId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,15 @@ import type {
FormFieldSize,
} from '~/src/components/Forms'

type LabelPosition = 'top' | 'left'

interface FormControlOptions {
labelPosition?: 'top' | 'left'
leftLabelWrapperHeight?: FormFieldSize
labelPosition?: LabelPosition
}

interface FormControlClassNameProps {
className: string
}

export interface FormControlContextCommonValue extends Partial<IdentifierProps> {}
Expand All @@ -22,10 +28,6 @@ export interface FormControlAriaProps {
'aria-describedby'?: string
}

interface WrapperProps {
Wrapper: React.FunctionComponent<ChildrenProps>
}

interface CallbackRefProps {
ref: (node: HTMLElement | null) => void
}
Expand All @@ -34,11 +36,11 @@ type PropsGetter<ExtraReturnType = {}> = <Props = {}>(props: Props) => Props & F

export type GroupPropsGetter = PropsGetter<CallbackRefProps & FormControlAriaProps>

export type LabelPropsGetter = PropsGetter<WrapperProps>
export type LabelPropsGetter = PropsGetter<FormControlClassNameProps>

export type FieldPropsGetter = PropsGetter<Omit<FormControlAriaProps, 'aria-labelledby'>>

export type HelperTextPropsGetter = PropsGetter<WrapperProps & CallbackRefProps & {
export type HelperTextPropsGetter = PropsGetter<FormControlClassNameProps & CallbackRefProps & {
visible: boolean
}>

Expand Down
Loading

0 comments on commit 0ec84a4

Please sign in to comment.