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

웹 접근성 개선과 토스트 문서 추가 #86

Merged
merged 9 commits into from
Nov 18, 2023
Merged

Conversation

hae-on
Copy link
Collaborator

@hae-on hae-on commented Nov 8, 2023

Issue

✨ 구현한 기능

토스트 문서 추가 했어요
Toast 메세지 뜨면 스크린리더가 바로 읽게 했어요
ESC 누르면 dialog 꺼지게 했어요
공백 유효성 검사 했어요 (일단 text area만 -> input에도 할깝쇼?)

  1. 공백만
  2. 띄어쓰기 연속 10번
  3. 줄바꿈 연속 4번
스크린샷 2023-11-09 오후 8 41 52

📢 논의하고 싶은 내용

ESC 누르면 꺼지게 하는 함수에서 이벤트에 type을 이렇게 줬거든요?

  const handleKeydown: KeyboardEventHandler<HTMLElement> = (e) => {
    if (e.keyCode === 27) {
      e.preventDefault();
      handleCloseBottomSheet();
    }
  };

이게 펀잇에서 Enter key custom hook 함수 로직인데 이렇게 정의했더라구요.
그래서 똑같이 했는데 이게 이 함수 자체에서는 오류가 없는데
useEffect 내부 함수를 넣으니까

스크린샷 2023-11-08 오후 11 58 04

이런 에러가 뜨면서 useEffect 내부에 'handleKeydown' 있는 곳 다 빨간 줄 나더라구요.
그래서 어쩔 수 없이 아래처럼 했는데 혹시 이유나 어떻게 바꿀 수 있는지 안다면 말해주세요.

구글링해도 안나오네요 후 ㅠㅠㅠ GPT는 any 이러고 이씀 ㅡㅡ

const handleKeydown = (e: KeyboardEvent) => {
    if (e.keyCode === 27) {
      e.preventDefault();
      handleCloseBottomSheet();
    }
  };

🎸 기타

x

@hae-on hae-on self-assigned this Nov 8, 2023
@Leejin-Yang
Copy link
Collaborator

이벤트 핸들러 타입 추측하는 바.. KeyboardEventHandler는 리액트에서 만든 타입이라 그런거 같아요
해온이 작성한대로 해도 좋습니다!

Copy link
Collaborator

@Leejin-Yang Leejin-Yang left a comment

Choose a reason for hiding this comment

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

수고했어요 해온~

@@ -16,7 +16,9 @@ const Toast = ({ id, message, isError = false }: ToastProps) => {

return (
<ToastWrapper isError={isError} isAnimating={isShown}>
<Message color={theme.colors.white}>{message}</Message>
<Message color={theme.colors.white} aria-live="assertive">
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@hae-on
Copy link
Collaborator Author

hae-on commented Nov 9, 2023

@Leejin-Yang
황펭아 유효성 검사 추가했어요 확인해주세요~

@Leejin-Yang
Copy link
Collaborator

@hae-on
유효성 검사 굳입니다 👍
그런데 사용처에서는 상태를 밖에서 만들고 prop으로 전달해주어야해서 유효성 검사 함수를 유틸로 제공하는건 어떤가요???

@hae-on
Copy link
Collaborator Author

hae-on commented Nov 9, 2023

@Leejin-Yang
제가 error 상태는 따로 빼서 사용할 예정인데요!
util 함수로 만들면 text area 자체에서 error를 export 해서 펀잇에서 사용할 수 있나요?!?!

Copy link
Collaborator

@Leejin-Yang Leejin-Yang left a comment

Choose a reason for hiding this comment

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

해온 다시 생각해봤는데 코멘트 확인해주세요!

value={value}
ref={ref}
resize={resize}
error={error}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 친구랑

Comment on lines 32 to 34
<Text size="xs" color={theme.colors.error} aria-live="assertive">
{error}
</Text>
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 친구는 여기 남기고

Comment on lines 4 to 22
const useSpaceValidator = (initialValue = '') => {
const [value, setValue] = useState(initialValue);
const [error, setError] = useState('');

const handleChange: ChangeEventHandler<HTMLTextAreaElement> = (e) => {
const newValue = e.target.value;
setValue(newValue);

const isWhitespaceOnly = !newValue.trim() && newValue.length > 2;
const isConsecutiveSpaces = /\s{10,}/.test(newValue);
const isConsecutiveNewlines = /\n{4,}/.test(newValue);

isWhitespaceOnly || isConsecutiveSpaces || isConsecutiveNewlines
? setError('의미 있는 리뷰를 작성해보는 건 어떨까요? 🫨')
: setError('');
};

return { value, error, handleChange };
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 훅은 사용처에서 작성해도 될거 같아요.
함수만 제공해도 될 거 같다고 했는데, 이건 사용처에서 처리하는 부분이라 common한 부분은 아닌거 같아서..! 아싸리 펀잇에서 작성하는건 어떤가요??

@@ -10,14 +14,33 @@ export interface TextareaProps extends ComponentPropsWithRef<'textarea'> {
}

const Textarea = ({ resize = 'both', ref, ...props }: TextareaProps) => {
const { value, error, handleChange } = useValidator();
Copy link
Collaborator

Choose a reason for hiding this comment

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

그럼 요거가 삭제되겠네요!

@hae-on
Copy link
Collaborator Author

hae-on commented Nov 12, 2023

@Leejin-Yang
이렇게 수정하는 거 맞을까요????
error라고 하니까 좀 안 와닿는 거 같은데 errorMessage로 바꾸는 건 어떻게 생각하시나요!!!!!!!!!!!!!!!!

@Leejin-Yang
Copy link
Collaborator

@hae-on
넵 좋습니다! 👍👍👍
errorMessage도 좋습니다.!

@hae-on
Copy link
Collaborator Author

hae-on commented Nov 12, 2023

@Leejin-Yang
얍 props 수정했고, 리드미 문서도 수정했습니다!
진짜 끝 -

@Leejin-Yang
Copy link
Collaborator

@hae-on
진짜 진짜 고생했습니다 해온 🌞

@hae-on hae-on merged commit fdcc8c2 into main Nov 18, 2023
2 of 3 checks passed
@hae-on hae-on deleted the feat/issue-84 branch November 18, 2023 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toast 문서 추가 웹 접근성 추가 dialog가 ESC로 꺼지지 않는 오류
2 participants