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

[FE] 접근성 : 넘버 키패드 스크린리더 개선 #749

Merged
merged 7 commits into from
Oct 17, 2024
Merged

Conversation

jinhokim98
Copy link
Contributor

@jinhokim98 jinhokim98 commented Oct 15, 2024

issue

개선 전 영상

KakaoTalk_20241015_160659836.mp4

현재는 위 영상에서 00 버튼을 '공' 버튼이라고 읽습니다.
실제로 0이 두 개이고 이 버튼을 클릭하면 1원에서 100원이 되지만 0이라고 읽힌다면 스크린리더로 서비스를 이용하는 사용자에게 불편한 경험을 줄 수 있습니다.

그래서 넘버 키패드 00을 '0' 2개 버튼으로 읽도록 개선해야합니다.

추가로 지우기 버튼도 다음보다 작음이라고 읽고 있습니다. 이를 금액 지우기, 버튼으로 읽어야 스크린리더 사용자가 헷갈리지 않고 이용할 수 있습니다.

구현 사항

keypad에 label 속성 추가

기존에는 amountKeypads 배열이 이렇게 선언되어 있었습니다.

const amountKeypads = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '00', '0', '<-'];

여기에 스크린 리더가 읽어야 할 label 속성을 추가했습니다.

  {
    keypad: '1',
    label: '1',
  },

스크린 리더가 읽는 값과 화면에 표시되는 값을 다르게 설정하는 방식으로 구현했습니다.
그래서 00은 0 2개 버튼, <-는 직전 입력 지우기로 설정했어요.

개선 후 영상

KakaoTalk_20241015_155007741.mp4

🫡 참고사항

스토리북에서 html lang = ko로 변경하는 방법

접근성 개선 결과를 확인하고 싶을 때 npm run storybook를 사용해 본인의 기기로 http://~~~:6006으로 확인을 주로 할 것이라 예상돼요. 이 때 스토리북의 preview 페이지는 우리가 설정한 index.html이 아니라 기본 lang 값이 en으로 설정돼있습니다. 그래서 스크린 리더가 한글을 읽을 때 굴려서 발음하게 됩니다.

레퍼런스를 찾아봤고 previewHead, preview-head.html을 작성하면 된다는 글이 있어 적용해봤지만 적용이 되지 않더라구요.
그래서 이를 변경하기 위해선 개선한 컴포넌트에 아래 두 문장 임의로 넣어서 테스트를 한 후 지워주면 됩니다.

const html = document.querySelector('html');
html?.setAttribute('lang', 'ko');

@jinhokim98 jinhokim98 added this to the v2.1.1 milestone Oct 15, 2024
@jinhokim98 jinhokim98 self-assigned this Oct 15, 2024
@jinhokim98 jinhokim98 linked an issue Oct 15, 2024 that may be closed by this pull request
2 tasks
Copy link
Contributor

@soi-ha soi-ha left a comment

Choose a reason for hiding this comment

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

접근성 개선을 빠른 시간안에 슥샥숑- 해버리다니... 역시 대쿠키

onClick: () => void;
disabled?: boolean;
}

export function Keypad({value, onClick, disabled = false}: Props) {
export function Keypad({value, label, onClick, disabled = false}: Props) {
Copy link
Contributor

Choose a reason for hiding this comment

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

label 추가한 건 아주 좋은 아이디어 같아요!
대신에 변수명을 조금 변경했으면 좋겠어요. 우리가 input과 같이 사용하는 label이라는 이름이 존재하기 때문에 혼동될 수 있을 것 같아요.
그래서 직관적으로 ariaLabel로 변경하는 건 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 확실히 그렇네요! label이라고 적으면 화면에 보이는 label이라고 생각할 수도 있을 것 같아요.
저도 이 부분이 고민인데 작업하실 때 접근성 관련 prop이 생긴다면 추가하는 변수명을 맞추면 좋을 것 같습니다.
일단 ariaLabel 좋은 것 같아요

@woowacourse-teams woowacourse-teams deleted a comment from github-actions bot Oct 16, 2024
Copy link

Copy link
Contributor

@Todari Todari left a comment

Choose a reason for hiding this comment

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

접근성의 신 대 쿠 키
고생 많았어요~!
Keypad prop들에 대해서 의견 하나 간단하게 남겨봤는데, 쿠키 의견이 궁금해서 RC 드려용~!!!

Comment on lines 8 to 19
interface Props {
value: string;
ariaLabel?: string;
onClick: () => void;
disabled?: boolean;
}

export function Keypad({value, onClick, disabled = false}: Props) {
export function Keypad({value, ariaLabel, onClick, disabled = false}: Props) {
const {theme} = useTheme();
return (
<button
aria-label={ariaLabel}
Copy link
Contributor

Choose a reason for hiding this comment

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

aria-label을 받아서 사용하는 것도 좋지만,
button prop들을 모두 받아와서 사용처에서 해당 버튼에 접근성 관련 attribute 뿐 아니라, 원하는 attribute를 자유롭게 넣어줄 수 있도록 하는건 어떨까요?
제가 작성했던, IconButton.tsx 의 타입과 활용을 한번 읽어보시고 자유롭게 의견주세용~!

(사실 이 코드도 내가 만든건데... 왜 그렇게 안했지...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

맞아요! 동의합니다. aria-label외에 다른 접근성 관련 prop이 더 생길 수도 있고, 다른 button의 prop이 필요한 경우도 있을 것 같아서 추가하는 것 좋은 것 같아요

},
{
keypad: '<-',
ariaLabel: '직전 입력 지우기',
Copy link
Contributor

Choose a reason for hiding this comment

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

지금도 전혀 문제가 없다고 생각합니다! 이부분은 반영해도 안해도 상관없습니다만,
백호가 의문을 가졌던, 직전 입력 지우기면 '00'을 눌렀을 때 두개를 지우지는걸로 기대할 수 있다는 생각에 어느정도 동의해요.
"가장 오른쪽 숫자 지우기", "마지막 숫자 지우기" 와 같은 식이면 어떨까요?
toss나 kakao는 어떻게 읽어주는지 참고해도 좋을 것 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

토스와 카카오 둘 다 단순 지우기라고 되어있네요!

저도 백호와 토다리의 의견에 동의합니다. 마지막 숫자 지우기로 바꿔도 좋을 것 같아요!

Copy link

Copy link
Contributor

@Todari Todari left a comment

Choose a reason for hiding this comment

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

변경된 것 확인했어요!!
고생 많았습니당 :)
그저 대 쿠 키

Copy link
Contributor

@pakxe pakxe left a comment

Choose a reason for hiding this comment

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

좋아욧! 고생많았습니다. 더 이상 이상한 이름으로 들리는 일이 없겠군요 👍

@@ -5,16 +5,16 @@ import {setDarker, setLighter} from '@components/Design/utils/colors';

import {Text, useTheme} from '@components/Design';

interface Props {
type KeypadProps = React.ButtonHTMLAttributes<HTMLButtonElement> & {
value: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

오? html button태그에는 value 속성이 없나보군요...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

html button 태그에 value가 있긴 합니다. 하지만 button 태그에 value prop을 넣어주지 않고 Text에 보여주는 역할을 하기 때문에 button prop의 value를 사용하지 않았어요.

Omit을 사용해서 html button의 value를 사용하지 않음을 명시해볼게요

Copy link
Contributor

Choose a reason for hiding this comment

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

아하 button에서 제공하는 value가 있긴 하지만 이 value가 아니라 직접 커스텀한 value를 사용한다는 의미였군요!! 👍

},
{
keypad: '00',
ariaLabel: '0 2개 버튼',
Copy link
Contributor

Choose a reason for hiding this comment

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

우와 좋네요. 이렇게 프로퍼티로 사용하는거!

다만 value, name이라는 이름도 괜찮을 것 같아요. keyPad라고 하면 화면에 출력되는 모습의 내용인건지, 입력했을 때 들어가는 실제 값인건지 헷갈릴 수 있지 않을까해소,,! value, name은 바로 실제 값, 화면 출력 값으로 인지되니까 어떨까 싶었어요. 하지만 이름이 어떠냐는 사실 이 pr의 핵심 기능이 아니라 부차적인 의견이므로 반영은 쿠키가 원하는대로 해주세요용

Copy link
Contributor Author

Choose a reason for hiding this comment

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

keypad가 화면에 보여지는 값, ariaLabel은 스크린 리더가 읽는 값이어서 name이라고 명명하면 조금 의미가 달라질 것 같아요ㅜ

Copy link
Contributor

@pakxe pakxe Oct 17, 2024

Choose a reason for hiding this comment

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

오 그렇기도 하겠네요. 좋아요좋아요 👍

ariaLabel: string;
};

export const amountKeypads: Keypad[] = [
Copy link
Contributor

Choose a reason for hiding this comment

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

amountKeypads, numberKeypads두 배열이 거의 비슷한 코드라서 반복된다고 느껴질 수 있을 것 같아요.

잠깐 생각해보기론, 기본적으로 1~9까지의 3*3배열과 키패드의 4행(1행이 시작)의 중간에 0이 있는 모습은 똑같으니까요. bottomLeft, bottomRight인자를 받아서 keypads 배열을 반환하는 함수는 어떤가요?
다만 이 경우 숫자를 987로 뒤집는 경우 조금의 분기문이 추가될 것 같다는 단점이 생각나는데요. 일단 지금은 123의 오름차순 순서를 사용하고 있고, 이 순서의 변경 가능성이 높진 않은 것 같아서 제안해봐욧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋아요~~ type마다 다른 키패드를 만들어주는 함수를 만들어서 개선해봤습니당

Copy link

@Todari Todari merged commit fd2338f into fe-dev Oct 17, 2024
2 checks passed
@Todari Todari deleted the feature/#748 branch October 17, 2024 06:24
@Todari Todari mentioned this pull request Oct 17, 2024
Todari pushed a commit that referenced this pull request Oct 17, 2024
* feat: 넘버 키패드 스크린리더로 사용했을 때 버튼 헷갈리지 않도록 label 설정

* style: label => ariaLabel으로 변수명 변경

* style: button element prop 확장해서 넘겨주는 방식으로 변경

* feat: button text를 읽은 후 text도 읽히지 않도록 aria-hidden 설정

* feat: 직전 입력 지우기 -> 마지막 숫자 지우기로 변경

* style: html button 내 value가 아닌 직접 정의한 value임을 명시

* refactor: 키패드 만드는 함수로 개선
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[FE] 접근성 : 넘버 키패드 스크린리더 개선
4 participants