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

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

Merged
merged 7 commits into from
Oct 17, 2024
+44 −17
Merged

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0f57696
feat: 넘버 키패드 스크린리더로 사용했을 때 버튼 헷갈리지 않도록 label 설정
jinhokim98 Oct 15, 2024
ad2d2ea
style: label => ariaLabel으로 변수명 변경
jinhokim98 Oct 16, 2024
86113b9
style: button element prop 확장해서 넘겨주는 방식으로 변경
jinhokim98 Oct 17, 2024
8281b53
feat: button text를 읽은 후 text도 읽히지 않도록 aria-hidden 설정
jinhokim98 Oct 17, 2024
7f6a3a8
feat: 직전 입력 지우기 -> 마지막 숫자 지우기로 변경
jinhokim98 Oct 17, 2024
1cf6116
style: html button 내 value가 아닌 직접 정의한 value임을 명시
jinhokim98 Oct 17, 2024
73bda7c
refactor: 키패드 만드는 함수로 개선
jinhokim98 Oct 17, 2024
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
16 changes: 8 additions & 8 deletions client/src/components/Design/components/NumberKeyboard/Keypad.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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를 사용한다는 의미였군요!! 👍

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

export function Keypad({value, onClick, disabled = false}: Props) {
export function Keypad({value, ...restButtonProps}: KeypadProps) {
const {theme} = useTheme();

return (
<button
{...restButtonProps}
css={css`
display: flex;
justify-content: center;
Expand All @@ -33,10 +33,10 @@ export function Keypad({value, onClick, disabled = false}: Props) {
}
}
`}
onClick={onClick}
disabled={disabled}
>
<Text size="title">{value}</Text>
<Text size="title" aria-hidden>
{value}
</Text>
</button>
);
}
14 changes: 7 additions & 7 deletions client/src/components/Design/components/NumberKeyboard/NumberKeyboard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {Button} from '@components/Design';

import {Keypad} from './Keypad';
import useNumberKeyboard from './useNumberKeyboard';
import {amountKeypads, numberKeypads} from './keypads';

export type KeyboardType = 'number' | 'string' | 'amount';

Expand All @@ -19,8 +20,6 @@ export interface NumberKeyboardProps {

export default function NumberKeyboard({type, maxNumber, initialValue, onChange}: NumberKeyboardProps) {
const {theme} = useTheme();
const amountKeypads = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '00', '0', '<-'];
const numberKeypads = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '', '0', '<-'];

const {onClickKeypad, onClickDelete, onClickDeleteAll, onClickAddAmount} = useNumberKeyboard({
type,
Expand Down Expand Up @@ -65,12 +64,13 @@ export default function NumberKeyboard({type, maxNumber, initialValue, onChange}
</Button>
</div>
)}
{(type === 'amount' ? amountKeypads : numberKeypads).map(el => (
{(type === 'amount' ? amountKeypads : numberKeypads).map(({keypad, ariaLabel}) => (
<Keypad
key={el}
value={el}
disabled={el === ''}
onClick={el === '<-' ? onClickDelete : () => onClickKeypad(el)}
key={keypad}
value={keypad}
aria-label={ariaLabel}
disabled={keypad === ''}
onClick={keypad === '<-' ? onClickDelete : () => onClickKeypad(keypad)}
/>
))}
</div>
Expand Down
105 changes: 105 additions & 0 deletions client/src/components/Design/components/NumberKeyboard/keypads.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
type Keypad = {
keypad: string;
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마다 다른 키패드를 만들어주는 함수를 만들어서 개선해봤습니당

{
keypad: '1',
ariaLabel: '1',
},
{
keypad: '2',
ariaLabel: '2',
},
{
keypad: '3',
ariaLabel: '3',
},
{
keypad: '4',
ariaLabel: '4',
},
{
keypad: '5',
ariaLabel: '5',
},
{
keypad: '6',
ariaLabel: '6',
},
{
keypad: '7',
ariaLabel: '7',
},
{
keypad: '8',
ariaLabel: '8',
},
{
keypad: '9',
ariaLabel: '9',
},
{
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 •
edited
Loading

Choose a reason for hiding this comment

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

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

},
{
keypad: '0',
ariaLabel: '0',
},
{
keypad: '<-',
ariaLabel: '마지막 숫자 지우기',
},
];
export const numberKeypads: Keypad[] = [
{
keypad: '1',
ariaLabel: '1',
},
{
keypad: '2',
ariaLabel: '2',
},
{
keypad: '3',
ariaLabel: '3',
},
{
keypad: '4',
ariaLabel: '4',
},
{
keypad: '5',
ariaLabel: '5',
},
{
keypad: '6',
ariaLabel: '6',
},
{
keypad: '7',
ariaLabel: '7',
},
{
keypad: '8',
ariaLabel: '8',
},
{
keypad: '9',
ariaLabel: '9',
},
{
keypad: '',
ariaLabel: '',
},
{
keypad: '0',
ariaLabel: '0',
},
{
keypad: '<-',
ariaLabel: '마지막 숫자 지우기',
},
];
Loading