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] input supportingText 구현 #41

Merged
merged 11 commits into from
Jul 8, 2024
4 changes: 2 additions & 2 deletions src/common/component/Header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ export const UserHeader = ({ isLogin = false }: HeaderProps) => {
<header css={headerStyle}>
<SmallLogo />
{isLogin ? (
<Button variant="secondary" size="xxSmall">
<Button variant="secondary" size="small">
로그아웃
</Button>
) : (
<Button variant="secondary" size="xxSmall">
<Button variant="secondary" size="small">
로그인
</Button>
)}
Expand Down
28 changes: 20 additions & 8 deletions src/common/component/Input/Input.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,23 @@ import { css } from '@emotion/react';
import { InputProps } from '@/common/component/Input/Input';
import { theme } from '@/common/style/theme/theme';

export const inputContainerStyle = css({
export const containerStyle = css({
display: 'flex',
flexDirection: 'column',

gap: '1.2rem',

width: '100%',
});

export const inputSupportStyle = css({
display: 'flex',
flexDirection: 'column',

gap: '0.8rem',
});

export const inputWarpperStyle = css({
export const warpperStyle = css({
display: 'flex',
alignItems: 'center',

Expand All @@ -28,17 +37,20 @@ export const inputStyle = css({

'::placeholder': {
color: theme.colors.gray_500,
...theme.text.body03,
},
});
Comment on lines 42 to 47
Copy link
Contributor

Choose a reason for hiding this comment

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

제가 스토리북으로 안 사실인데, input안의 텍스트에 대한 폰트 사이즈와 라인 헤이트도 설정해주셔야 될 것 같습니다 ! GUI 보구요 !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오오! 알려주셔서 넘 감사합니다! 수정완료했습니다 !! 💯


export const variantStyle = (variant: Required<InputProps>['variant']) => {
export const variantStyle = ({ variant, isError }: { variant: Required<InputProps>['variant']; isError: boolean }) => {
const borderColor = isError ? `${theme.colors.red}` : `${theme.colors.gray_400}`;

const style = {
outline: {
boxShadow: ` 0px 0px 0px 1px ${theme.colors.gray_400}`,
boxShadow: ` 0px 0px 0px 1px ${borderColor}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

요거 inset 추가해주어야 하지 않나요 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

inset 안하면 원래의 경계선에 shadow가 생기고 inset을 설정하면 경계선 안에 shadow가 생기더라구요!
underline 상태도 inset을 하지 않았기 때문에 outline도 inset설정하지 않았습니당

Copy link
Contributor

Choose a reason for hiding this comment

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

outline의 취지가 안에 border처럼 선이 생길려고 하는 의도가 맞지 않나요 ?? 지금처럼 작성해주면 말 그대로 box에 흐릿한 그림자가 생기는 것이라input의 의도상 inset이 맞지 않나 싶네요 !

그리고 추가적인 의견인데 outline 보다는 default 어떤가요 ? 저희 서비스 특징 상 input의 기본적인 형상이 outline이기 때문에 의견 한 번 내봅니다 !

Copy link
Contributor

Choose a reason for hiding this comment

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

그리고 현재 boxShadow에 할당한 스트링에 맨 앞 공백 다 제거해주시면 감사하겠습니다 !

borderRadius: '8px',
},
underline: {
boxShadow: ` 0px 1px 0px ${theme.colors.gray_400}`,
boxShadow: ` 0px 1px 0px ${borderColor}`,
},
colored: {
borderRadius: '100px',
Expand All @@ -51,9 +63,9 @@ export const variantStyle = (variant: Required<InputProps>['variant']) => {

export const sizeStyle = (size: Required<InputProps>['size']) => {
const style = {
small: { padding: '0.8rem 1.2rem', ...theme.text.body04 },
medium: { padding: '1.2rem 1.2rem', ...theme.text.body02 },
large: { padding: '1.2rem 1.6rem', ...theme.text.body02 },
small: { padding: '0.8rem 1.2rem' },
medium: { padding: '1.2rem 1.2rem' },
large: { padding: '1.2rem 1.6rem' },
};

return style[size];
Expand Down
30 changes: 23 additions & 7 deletions src/common/component/Input/Input.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import React, { InputHTMLAttributes } from 'react';

import {
inputContainerStyle,
containerStyle,
inputStyle,
inputWarpperStyle,
inputSupportStyle,
sizeStyle,
variantStyle,
warpperStyle,
} from '@/common/component/Input/Input.style';
import Label from '@/common/component/Label/Label';
import SupportingText from '@/common/component/SupportingText/SupportingText';

type InputSize = 'small' | 'medium' | 'large';
type InputVariant = 'outline' | 'underline' | 'colored';
Expand All @@ -18,15 +20,29 @@ export interface InputProps extends Omit<InputHTMLAttributes<HTMLInputElement>,
label?: string;
LeftIcon?: React.FunctionComponent<React.SVGProps<SVGSVGElement>>; //svg 컴포넌트
isError?: boolean;
isNotice?: boolean;
supportingText?: string;
}

const Input = ({ variant, size = 'medium', label, LeftIcon, ...props }: InputProps) => {
const Input = ({
variant,
Copy link
Member

Choose a reason for hiding this comment

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

스타일 적용한 거 보니까 Required로 variant 값을 가져왔던데, 그렇다면 variant 값을 옵셔널로 주는 방향으로 코드를 작성하는 게 맞는 것 같습니다 !

size = 'medium',
label,
LeftIcon,
isError = false,
isNotice = false,
supportingText,
...props
}: InputProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

특히 Input 컴포넌트는 forwardRef를 통해서 외부에서 ref를 전달받을 수 있도록 구현하는 것이 정말 좋아요 ! 왜냐하면 input이란 요소 자체가 ref를 통해 focusvalue 관련 기능을 많이 필요로 하기 때문입니다.

forwardRef는 말 그대로 부모 컴포넌트에게 ref를 전달할 수 있게끔 해주는 녀석이에요. Input은 순수 html 태그로 이루어진 jsx 요소가 아니기 때문에 원래는 ref를 전달하지 못하지만 다음과 같이 forwardRef를 사용하여 ref를 전달할 수 있어요.

const Input = ({
  ...
  
}: InputProps, ref: ForwardRef<HTMLInputElement>) => {
  return <input ref={ref} />;
}

export default forwardRef(Input)

이제 해당 Input 컴포넌트를 사용할 때도, 외부에서 ref를 prop으로 전달할 수 있게 되면서 하나의 DOM 노드로서 기능할 수 있도록 해준답니다 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵! 알려주셔서 감사합니다~~!!
수정하였습니다!! 👍

return (
<article css={inputContainerStyle}>
<article css={containerStyle}>
{label && <Label id={label}>{label}</Label>}
<div css={[inputWarpperStyle, variantStyle(variant), sizeStyle(size)]}>
{LeftIcon && <LeftIcon />}
<input css={[inputStyle]} {...props} />
<div css={inputSupportStyle}>
<div css={[warpperStyle, variantStyle({ variant, isError }), sizeStyle(size)]}>
{LeftIcon && <LeftIcon />}
<input css={inputStyle} {...props} />
</div>
{supportingText && <SupportingText text={supportingText} isError={isError} isNotice={isNotice} />}
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서 SupportingText한테 텍스트는 children으로 넘겨준다면 더 깔끔할 것 같다고 생각이 들어요 !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이말을 듣고 children에 대해서 알아보고 적용하였습니다!
children이 사용하기 훨씬 편할 것 같다는 생각이 드네욤 🙇‍♀️
좋은 지적 감사합니다 💟

</div>
</article>
);
Expand Down
2 changes: 1 addition & 1 deletion src/common/component/Modal/Modal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ReactElement, useEffect, useRef } from 'react';
import { ReactElement, useEffect } from 'react';
import ReactDOM from 'react-dom';

import { backgroundStyle, dialogStyle } from '@/common/component/Modal/Modal.style';
Expand Down
2 changes: 1 addition & 1 deletion src/common/component/Select/Select.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ export const itemStyle = css({
lineHeight: theme.text.body04.lineHeight,

'&:hover, &:focus': {
backgroundColor: theme.colors.blue_10,
backgroundColor: theme.colors.blue_100,
},
});
6 changes: 6 additions & 0 deletions src/common/component/SupportingText/SupportingText.style.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { css } from '@emotion/react';

import { theme } from '@/common/style/theme/theme';

export const textStyle = (textColor: string) =>
css({ color: textColor, wordBreak: 'break-word', ...theme.text.body04 });
15 changes: 15 additions & 0 deletions src/common/component/SupportingText/SupportingText.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { textStyle } from '@/common/component/SupportingText/SupportingText.style';
import { theme } from '@/common/style/theme/theme';

interface SupportingTextProps {
text: string;
isError?: boolean; //red
isNotice?: boolean; //blue
Copy link
Contributor

Choose a reason for hiding this comment

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

주석을 사용하실거라면 저렇게 red, blue 보다는 해당 prop이 어떤 용도로 쓰이는지를 적어주시는게 더 나을 것 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵! 이름에서 용도가 드러난다고 판단해서 주석 삭제하였습니당

}

const SupportingText = ({ text, isError = false, isNotice = false }: SupportingTextProps) => {
const textColor = isError ? theme.colors.red : isNotice ? theme.colors.blue_900 : theme.colors.gray_400;
return <p css={textStyle(textColor)}>{text}</p>;
Copy link
Contributor

Choose a reason for hiding this comment

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

요론거 가독성 좋게 return 문은 한 칸 띄워주시면 어떨지..!

그리고 isErrorisNoticetextStyle 함수에 바로 넘겨주고, 색깔 정하는 삼항연산자는 스타일 함수 내에서 수행하게 하는 건 어떠신가요 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵!
textStyle에 많은 인자를 넣는 게 깔끔해보이지 않아서 저렇게 밖에서 했는데
주용님 말 들어보니까 스타일은 스타일 함수에서 수행하게 하는 것이 더 깔끔한 방법일 것 같네요!!! 👍
감사합니다 !!

};

export default SupportingText;
1 change: 0 additions & 1 deletion src/story/common/Modal.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Meta, StoryObj } from '@storybook/react';

import Modal from '@/common/component/Modal/Modal';
import { useOutsideClick } from '@/common/hook';
import useModal from '@/common/hook/Modal/useModal';

const meta: Meta<typeof Modal> = {
Expand Down
2 changes: 1 addition & 1 deletion src/story/page/MonthHeader.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const meta = {
layout: 'centered',
},
args: {
onMonthClick: (month: string) => {},
onMonthClick: () => {},
},
argTypes: {},
} satisfies Meta<typeof MonthHeader>;
Expand Down