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

[접근성개선] 스크린 리더가 태그 등록 메시지 안내 #727

Merged
merged 5 commits into from
Oct 8, 2024

Conversation

vi-wolhwa
Copy link
Contributor

⚡️ 관련 이슈

#722

📍주요 변경 사항

1. 스크린리더를 위한 컴포넌트 구현

2. 태그 등록 시, 스크린리더 메시지 출력

🎸기타

태그 등록 시, 다음과 같은 메시지 출력

  1. "[TagName] 태그 등록"
  2. "[TagName]"
  3. "Deleted"
    모바일로 로그인이 불가능하여 정확한 디버깅이 어려움

@vi-wolhwa vi-wolhwa added feature 기능 추가 FE 프론트엔드 labels Oct 2, 2024
@vi-wolhwa vi-wolhwa added this to the 6차 스프린트 🦴 milestone Oct 2, 2024
@vi-wolhwa vi-wolhwa self-assigned this Oct 2, 2024
@Hain-tain Hain-tain changed the title 스크린 리더가 태그 등록 메시지 안내 [접근성개선] 스크린 리더가 태그 등록 메시지 안내 Oct 4, 2024
@vi-wolhwa
Copy link
Contributor Author

(10/07 16:15)

  1. 마위의 피드백에 따라 기능을 훅으로 분리하였습니다.
  2. 각 컴포넌트에서 스크린리더 요소(div)를 분리하기 위해 RouterProvider 외부로 분리하였습니다.
  3. aria, style 속성을 갖는 스크린리더 요소를 컴포넌트화 하였습니다.

@vi-wolhwa vi-wolhwa closed this Oct 7, 2024
@vi-wolhwa vi-wolhwa reopened this Oct 7, 2024

export const useScreenReader = () => {
const elementRef = useRef<HTMLDivElement | null>(null);
const updateScreenReaderMessage = useCallback((message: string) => {
const element = document.getElementById('screen-reader');
Copy link
Contributor

Choose a reason for hiding this comment

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

한 페이지에 screen-reader 라는 id를 가진 DOM요소가 한 개 이상 있다면 메시지가 중복으로 읽히지 않을까라는 우려가 들긴 하네요..! 혹시 해당 부분 테스트가 완료 되었나요?!

Copy link
Contributor Author

@vi-wolhwa vi-wolhwa Oct 7, 2024

Choose a reason for hiding this comment

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

한 화면에 동일한 id를 갖는 요소는 존재할 수 없으니, 개발자의 실수로 생성했다면 에러가 나지 않을까 생각합니당

Copy link
Contributor

Choose a reason for hiding this comment

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

document.getElementById() 메소드는 첫 번째로 찾은 요소만 반환하기 때문에 에러가 날 일은 없을 듯 싶습니다..! 다만 원하는대로 동작하게 하기 위해서는 올바르게 <ScreenReader /> 를 찾을 수 있도록 다른 곳에서 id 사용에 유의해야겠네요..!!

덧붙여 혹시 조금 더 리액트스럽게 ref 를 사용해보는 방향은 어떨까요?


export const useScreenReader = () => {
const elementRef = useRef<HTMLDivElement | null>(null);
const updateScreenReaderMessage = useCallback((message: string) => {
const element = document.getElementById('screen-reader');
Copy link
Contributor

Choose a reason for hiding this comment

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

document.getElementById() 메소드는 첫 번째로 찾은 요소만 반환하기 때문에 에러가 날 일은 없을 듯 싶습니다..! 다만 원하는대로 동작하게 하기 위해서는 올바르게 <ScreenReader /> 를 찾을 수 있도록 다른 곳에서 id 사용에 유의해야겠네요..!!

덧붙여 혹시 조금 더 리액트스럽게 ref 를 사용해보는 방향은 어떨까요?

@@ -49,6 +50,7 @@ enableMocking().then(() => {
<HeaderProvider>
<GlobalStyles />
<RouterProvider router={router} />
<ScreenReader />
Copy link
Contributor

Choose a reason for hiding this comment

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

오호... 혹시 스크린리더 컴포넌트를 최상위에 둔 이유가 있나요?

현재 월하의 리펙토링을 보면 최상위에 <ScreenReader /> 라는 눈에 보이지 않는 컴포넌트를 두고, 만약 스크린 리더기가 읽어줬으면 하는 내용이 생기면 그 때 useScreenReader 에서 업데이트를 해서 aria-live: polite 로 읽게 해주셨군요!

마치 Toast 처럼 스크린리더 내용을 바꿔껴준다는 생각이 듭니다. 이런 방향은 생각하지 못한 방향이어서 굉장히 신선하네요!

다만 이렇게 전역적으로 사용하였을 때, 유의해야 할 점이 어떤게 있을지 함께 생각해봐도 좋을 것 같아요.

일단 제가 가장 먼저 생각나는 것은

  1. 긴 태그를 등록했을 때, 다음 태그 등록을 굉장히 빨리한다면? 즉 이전 태그 등록을 읽어주는 와중에 updateScreenReaderMessage 가 불려버린다면 어떻게 될까? 원하는대로 이전 태그를 모두 읽어주고 다음 태그를 읽어줄까?
  2. 만약 이렇게 전역적으로 관리되면 토스트가 떴을 때에도 이 로직을 사용해볼 수 있을 것 같은데, 태그 등록을 읽어주고 있는 와중에 저장 버튼을 눌러 오류 메세지가 읽힌다면?

등등 하나의 상태로 관리해도 우리가 원하는대로 잘 동작하는지, 이렇게 스크린 리더가 읽어줘야 할 부분들을 전역으로 관리하는 것이 괜찮을지 함께 이야기 나눠봐도 좋을 것 같아요!!

그런데 다시 한 번 말하지만 이렇게 전역에서 두는 것도 굉장히 신선한 아이디어네요!! 각 컴포넌트 내부에 별도의 태그들을 만들지 않아도 되는 점이 좋은 것 같아요 👍👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. 현재 메시지를 읽는 방법은 새로운 태그가 등록되면 다음 메시지를 즉각적으로 읽어줍니다. 이전 요소를 읽는 중에 다음 요소를 포커스 했을 때와 동일하게 동작합니다. 반드시 태그 전체를 읽어줄 필요는 없다고 생각합니다.
  2. 이 부분은 고려하지 않아도 괜찮을 것 같다는 생각이 듭니다. 그 이유는 스크린리더 사용자가 스크린리더의 음성을 듣지도 않고 저장 버튼을 클릭하는 상황이 있을까 하는 문제라고 생각합니다. 이를 고려하더라도 (1)에서와 같이 반드시 태그를 읽어주기 위해 에러메시지를 무시하는 것은 오히려 접근성을 해치는 방식이라고 생각합니다. 에러메시지로 음성이 전환되는 것이 자연스럽다고 생각합니당

전역 상태로 관리한 이유는 스크린 리더를 사용하는 컴포넌트마다

요소를 렌더링해야 하는데, 전역에서 관리하면 이를 회피할 수 있고, RenderProvider의 리렌더링에 영향을 미치지 않기 때문에 성능적으로도 문제 없다고 생각했기 때문입니다.

Copy link
Contributor Author

@vi-wolhwa vi-wolhwa Oct 7, 2024

Choose a reason for hiding this comment

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

Ref를 사용하는 방법도 생각해보았는데, 전역으로 관리한다면 Ref를 context 없이 사용하기 어렵다고 생각하여서 이렇게 구현하였습니다ㅠㅠ 이 부분에 대해서 고견 듣고 싶습니다..

Copy link
Contributor

Choose a reason for hiding this comment

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

ref 를 쓰게되면 아마 context 가 또 생기거나, 아니면 인자로 받아서 써야할텐데...

후자처럼 useScreenReader 훅에서 ref를 받아 메시지를 업데이트하는 방식으로 바꾸려면 아래와 같이 사용할 수 있다고 하네요..! (feat. 지선생님)

import React, { forwardRef } from 'react';

const ScreenReader = forwardRef((props, ref) => (
  <div
    ref={ref}
    id='screen-reader'
    aria-live='polite'
    aria-hidden='true'
    style={{
      position: 'absolute',
      width: '1px',
      height: '1px',
      margin: '-1px',
      padding: '0',
      border: '0',
      overflow: 'hidden',
      clip: 'rect(0, 0, 0, 0)',
      whiteSpace: 'nowrap',
      wordWrap: 'normal',
    }}
  />
));

export default ScreenReader;
 const screenReaderRef = useRef();
 const { updateScreenReaderMessage } = useScreenReader(screenReaderRef);

그런데 이렇게 ref를 매번 생성해서 전달해주는 부분이 불필요하게 느껴진다는 점에서 좋은 변화는 아니라는 생각도 듭니다...😂😂

저도 컨텍스트를 만들어서 쓰는게 아니라면 더 좋은 생각이 나지 않네요...
일단 지금은 이렇게 두고 추후 더 좋은 생각이 나면 리팩토링 해보는 것으로 할까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

너무 작고 사소한 제안인데, 이름 ScreenReaderOnly는 어떤가요..?ㅎㅎ 만약 바꾸게된다면 훅 이름도 함께 useScreenReaderOnly로 바꿔도 좋을 것 같아요..!

지금 이름도 충분히 스크린 리더를 위한 것이라는 느낌이 들지만, only를 붙여서 눈에는 보이지 않고 오로지 스크린 리더기를 위한 것이라는 것을 조금 더 강조하고 싶어서 제안해봅니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이 부분은 바로 반영하도록 하겠습니당

@Jaymyong66 Jaymyong66 merged commit 010413d into dev/fe Oct 8, 2024
3 checks passed
@Jaymyong66 Jaymyong66 deleted the refactor/722-tag-registration-announcement branch October 8, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 feature 기능 추가
Projects
Status: Weekend Done
Development

Successfully merging this pull request may close these issues.

3 participants