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

[REFACTOR] 접근성 컴포넌트 A11yOnly 리팩토링 및 추가 접근성 코드 리팩토링 #362

Merged
merged 10 commits into from
Oct 23, 2024

Conversation

useon
Copy link
Contributor

@useon useon commented Oct 22, 2024

Issue Number

#361

As-Is

  1. role="text"는 표준으로 정의되어 있지 않은 속성인데 임의로 사용하고 있다.
  2. 몇 가지 컴포넌트에서 접근성 코드의 개선이 필요하다.

To-Be

A11yOnly

  • role 속성을 정의하지 않으면 text를 기본으로 설정한 코드를 삭제
  • 사용처에서 필요한 수정

게임 타이머

  • role="alert" 속성을 추가한다. 암묵적으로 aria-live="assertive", aria-atomic=true를 포함한다.

status가 아니라 alert를 사용한 이유는 1) 매 초 남은 시간을 안내하는 것이 아니라 제한 시간 시작, 중간, 5초 남았을 때에 안내하기 때문이다. 2) 선택지를 제한 시간안에 골라야 하기 때문에 긴급한 알림이라고 판단하였다.

대기 방 인원

  • role="status" 속성을 추가한다. 암묵적으로 aria-live="polite", aria-atomic=true를 포함한다.

게임 선택지

  • 선택지 두 개를 감싸는 태그를 role="radiogroup"을 설정하고 하단 버튼을 role="radio"로 설정, aria-checked를 추가하여 선택됨 또는 선택되지 않음을 알려준다.

두 개 중 하나를 선택하는 것이라 라디오의 성격을 가진다고 생각했다.

Check List

  • 테스트가 전부 통과되었나요?
  • 모든 commit이 push 되었나요?
  • merge할 branch를 확인했나요?
  • Assignee를 지정했나요?
  • Label을 지정했나요?

Test Screenshot

(Optional) Additional Description

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/alert_role
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/status_role
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/radio_role

🌸 Storybook 배포 주소

https://woowacourse-teams.github.io/2024-ddangkong/storybook/

@useon useon added ♻️ refactor 리팩토링 🫧 FE front end labels Oct 22, 2024
@useon useon added this to the FE Sprint6 milestone Oct 22, 2024
@useon useon self-assigned this Oct 22, 2024
@useon useon linked an issue Oct 22, 2024 that may be closed by this pull request
1 task
Copy link
Contributor

@rbgksqkr rbgksqkr left a comment

Choose a reason for hiding this comment

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

오 좋은데요 디테일한 수정 감사합니다 접근성 마스터 썬데이 그는 빛이 납니다 하하하호호✨✨✨✨✨✨✨✨✨✨

@@ -25,7 +25,7 @@ describe('SelectContainer', () => {

customRender(<SelectContainer />);

const optionButton = await screen.findByRole('button', { name: SELECT_OPTION });
const optionButton = await screen.findByRole('radio', { name: SELECT_OPTION });
Copy link
Contributor

Choose a reason for hiding this comment

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

🌸 칭찬 🌸

지렸다 radiogrup하고 radio 하면 몇번째 중 몇번째가 떠서 좋은 것 같아요!

@@ -31,6 +31,7 @@ const Timer = ({ selectedId, isVoted, completeSelection }: TimerProps) => {
isVoted,
completeSelection,
});
const ScreenReaderLeftRoundTime = `${leftRoundTime}초 남았습니다.`;
Copy link
Contributor

Choose a reason for hiding this comment

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

💭 질문 💭

카멜 케이스가 아닌 파스칼 케이스를 사용한 이유가 있을까요?

const ScreenReaderLeftRoundTime = `${leftRoundTime}초 남았습니다.`;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

허걱쓰 ! 수정하고 머지하겠습니다 ^ .^

css={SelectOptionLayout(selectedId === option.optionId, isCompleted)}
onClick={() => handleClickOption(option.optionId)}
disabled={isCompleted}
aria-pressed={selectedId === option.optionId}
aria-checked={selectedId === option.optionId}
Copy link
Contributor

Choose a reason for hiding this comment

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

🌸 칭찬 🌸

오우 radio로 바꾸니까 checked로 되는군요 굿굿

Copy link
Contributor

@novice0840 novice0840 left a comment

Choose a reason for hiding this comment

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

확인했습니다!
바로 approve 할게요!

@useon useon merged commit 28fc3b4 into develop Oct 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🫧 FE front end ♻️ refactor 리팩토링
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[REFACTOR] 접근성 컴포넌트 A11yOnly 리팩토링
3 participants