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.

Sign up for GitHub

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

Jump to bottom

feature/FE-052 : IconButton disabled 에러 수정 #71

Merged
merged 2 commits into from
Jul 4, 2023
Merged

feature/FE-052 : IconButton disabled 에러 수정 #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a602a5c
chore: button disbled 이슈 해결
naro-Kim Jul 3, 2023
d52ffe4
chore: minus-hover sprite 교체
naro-Kim Jul 3, 2023
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
8 changes: 8 additions & 0 deletions public/sprite.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 8 additions & 0 deletions public/spriteIcons/minus-hover.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions src/app/write/components/Counter/Counter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ const Counter = ({ control, name, min = 2, max = 20, ...rest }: CounterProps<any
name={name}
render={({ field: { value, onChange } }) => (
<>
<IconButton type="button" iconType="minus" onClick={handleClickSubtract} disabled={value <= 2} />
<IconButton type="button" iconType="minus" onClick={handleClickSubtract} disabled={value <= 2} hover />
<input type="number" min={min} max={max} value={value} onChange={onChange} />
<IconButton type="button" iconType="plus" onClick={handleClickAdd} />
<IconButton type="button" iconType="plus" onClick={handleClickAdd} hover />
</>
)}
/>
Expand Down
1 change: 1 addition & 0 deletions src/app/write/components/Form/FirstStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const FirstStep = ({ nextStep }: stepProps) => {
const { stepOne, setData } = useFormStore();

const method = useForm<StepOneSchema>({
mode: 'onSubmit',
resolver: zodResolver(stepOneSchema),
defaultValues: stepOne || {},
});
Expand Down
6 changes: 5 additions & 1 deletion src/components/IconButton/IconButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,13 @@ const IconButton = ({

useEffect(() => {
if (!error) return;
setStatus(error ? ICON_STATUS.ERROR : disabled ? ICON_STATUS.DISABLE : ICON_STATUS.DEFAULT);
setStatus(error ? ICON_STATUS.ERROR : ICON_STATUS.DEFAULT);
}, [error, disabled]);

useEffect(() => {
setStatus(disabled ? ICON_STATUS.DISABLE : ICON_STATUS.DEFAULT);
}, [disabled]);

Comment on lines 43 to +51
Copy link
Member

Choose a reason for hiding this comment

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

IconButton 내에서 useEffect 가 많아져서 로직 파악이 어려워지고 있는 것 같아요..!
나중에 기회될때 useEffect 를 없애는 방향으로 리팩토링 진행해보면 좋을 것 같아요~ state와 props에 변화에 대응하기 위해 useEffect를 사용하는 것은 좋지 않은 패턴이라고 하네요,,!

const handleMouseEnter = () => {
if (error || disabled || status === ICON_STATUS.ACTIVE) return;
hover && setStatus(ICON_STATUS.HOVER);
Expand Down
14 changes: 7 additions & 7 deletions src/components/SvgIcon/SvgIcon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { SVGProps } from 'react';

interface Props extends SVGProps<SVGSVGElement> {
/** icon id (spriteIcons 폴더하에 있는 svg 파일명과 동일함) */
id: string;
/** icon id (spriteIcons 폴더하에 있는 svg 파일명과 동일함) */
id: string;
}

const SvgIcon = ({ id, width = 16, height = 16, fill = 'none', ...rest }: Props) => {
return (
<svg width={width} height={height} fill={fill} {...rest}>
<use href={`/sprite.svg#${id}`} />
</svg>
);
return (
<svg width={width} height={height} fill={fill} {...rest}>
<use href={`/sprite.svg#${id}`} />
</svg>
);
Comment on lines +9 to +13
Copy link
Member

Choose a reason for hiding this comment

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

마이너스 아이콘이 호버 효과가 반영 안되는 문제가 있어보이는데 확인해보니 minus-hover.svg 아이콘이 잘못된 것으로 보입니다! 하는 김에 수정해주시고 sprite.svg에도 반영해주시면 감사하겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 파일이 잘못되었나 싶어서 re-export한다음 피그마에 띄워 이상이 없는걸 확인해보고 sprite 생성을 했었는데.. 온전히 다 삭제하고 한번 다시 봐보겠습니다~!

};

export default SvgIcon;