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

[항공사 웹사이트의 컴포넌트 접근성 높이기] 썬데이(김유선) 미션 제출합니다. #137

Merged
merged 20 commits into from
Oct 4, 2024

Conversation

useon
Copy link

@useon useon commented Sep 30, 2024

🍪 쿠키에게

안녕하세요 쿠키!!!! 첫 페어로 만난지 벌써 7개월이 흘렀네요 ^^ .. 그때는 어색해서 왔다감 사진도 멀리 떨어져 찍은 기억이 있네요 푸하하. 이렇게 다시 만나게 되어 너무 기쁘네요 !!!!!! 즐거운 리뷰 시간을 보내봅시다 ^ .^ 급하게 PR 작성하느라 엄청 자세히 적지는 못했네요 궁금하신 부분 코멘트 남겨 주세요 🌞🌞🌞

🔥 결과

✅ 개선 작업 목록

1 컴포넌트 접근성 개선 - 이미지 캐로셀

  • 스크린 리더가 캐로셀의 전체 아이템 수를 읽을 수 있어야 합니다.

    • 시각적으로 보이지는 않지만 스크린 리더기에 읽힐 수 있도록 요소를 추가하였습니다 ^. ^
      <div className="visually-hidden">
         {`세계 여행 상품 ${travelOptions.length} 중 ${currentIndex + 1}번째 상품`}
      </div>
    
     // 제공하는 스타일 사용
     .visually-hidden {
     position: absolute;
     width: 1px;
     height: 1px;
     padding: 0;
     margin: -1px;
     overflow: hidden;
     clip: rect(0, 0, 0, 0);
     white-space: nowrap;
     border: 0;
    }
    
  • 스크린 리더가 이미지 캐로셀 내 각 아이템 정보를 읽을 수 있어야 합니다.

    • 여행지, 좌석 유형, 가격 정보를 한번에 읽을 수 있어야 합니다.
    • 이전/다음 아이템으로 이동하고 현재 보이는 아이템의 정보를 읽을 수 있어야 합니다.
      이 부분 구현을 하면서 원하는 대로 읽혀지게 하기 위해 여러 번 시도를 했습니다 .. 계속 '세계 여행 상품 ${travelOptions.length} 중 ${currentIndex + 1}번째 상품' 까지만 읽더라고요. 결국 두 요소를 감싸는 요소에 aria-live='polite'를 사용하니 두 요소를 모두 읽어주어서 이렇게 개선하였습니다 ^^ ! 또한 원래 순서대로라면 이전 상품 버튼 -> 상품 정보 -> 다음 상품 버튼 순서였는데, 상품 정보 -> 이전 상품 버튼 -> 다음 상품 버튼으로 수정했습니다. 스타일이 애초에 순서를 변경해도 그대로 적용되는 상태였고, 순서 상 상품 정보를 먼저 안내하고 버튼으로 이동할 수 있는 것이 편할 것 같다고 생각했어요.
    <article className={styles.travelSection}>
    <div className={styles.carousel} aria-live="polite">
      <div className="visually-hidden">
        {`세계 여행 상품 ${travelOptions.length}${currentIndex + 1}번째 상품`}
      </div>
      {travelOptions.map((option, index) => (
        <button
          key={index}
          className={`${styles.card} ${index === currentIndex ? styles.cardActive : ''}`}
          onClick={() => handleCardClick(option.link)}
          aria-label={`${option.departure}출발${option.destination}도착 ${
            option.type
          } 가격${option.price.toLocaleString()}원 선택하면 예약 페이지로 이동합니다.`}
        >
          <img src={option.image} className={styles.cardImage} alt="" />
          <div className={styles.cardContent}>
            <p className={`${styles.cardTitle} heading-3-text`}>
              {option.departure} - {option.destination}
            </p>
            <p className={`${styles.cardType} body-text`}>{option.type}</p>
            <p className={`${styles.cardPrice} body-text`}>KRW {option.price.toLocaleString()}</p>
          </div>
        </button>
      ))}
    </div>
    <button
      className={`${styles.navButton} ${styles.navButtonPrev}`}
      onClick={prevTravel}
      aria-label="이전 여행 상품"
    >
      <img src={chevronLeft} className={styles.navButtonIcon} />
    </button>
    <button
      className={`${styles.navButton} ${styles.navButtonNext}`}
      onClick={nextTravel}
      aria-label="다음 여행 상품"
    >
      <img src={chevronRight} className={styles.navButtonIcon} />
    </button>
  • 각 아이템을 선택하면 각각에 맞는 링크로 이동할 수 있어야 합니다.

2 페이지 접근성 개선

  • 페이지를 하나의 문서로 읽을 수 있어야 합니다.

    • 페이지에 적절한 제목(title)을 제공하세요. 제목은 페이지의 주요 내용을 간결하게 설명해야 합니다.
    • 페이지의 주요 영역을 시맨틱 태그를 사용해 명확히 구분해 주세요
    • 헤딩을 논리적인 순서로 사용해 페이지 구조를 명확히 해주세요
      저번 사전 미션 코드를 들고 왔는데요. 시간 나시면 요기 PR도 읽어보셔요 ~! !
      [사전 미션 - 워밍업] - 썬데이(김유선) 미션 제출합니다. self-paced-enhance-usability#30
      사전 미션 요구 사항 이외에도 추가적으로 구현한 부분도 있습니다. (최소 상태일 때 감소를 누를 때마다, 최대 상태일 때 증가
      를 누를 때마다 최소/최대 수에 도달하였다고 안내하기)
  • 키보드 사용자를 위해 페이지 최상단에 '본문으로 바로가기' 링크를 제공해 반복되는 메뉴를 건너뛸 수 있게 해주세요

    • 개선 전
    default.mp4
    • 개선 후
    default.mp4

    탭키를 눌렀을 때 왜 Skip to content 링크가 가장 먼저 뜨는 이유는 탭 순서와 관련된 웹 접근성 규칙 때문이라고 하네요 ! 지금 상위에 SkipToContent 컴포넌트가 위치해서 해당 작업이 탭을 눌렀을 때 가장 먼저 실행되는 것 같아요. 저는 항공권 예매 섹션 안에 물음표 툴팁으로 바로 넘어가게 되어서 항공권 예매에 tabIndex 속성을 넣어 해당 부분으로 넘어가도록 작성했습니다.

3 팝업 모달 포커스 트랩

  • 스크린 리더로 진입했을 때에도 현재 팝업이 떴다는 것을 인지할 수 있게 개선해 주세요.
    aria-live의 assertive 속성을 이용했고, visually-hidden을 사용하여 모달이 뜨는 경우 모달 창이 열렸다는 음성을 바로 안내하도록 작성했습니다.
    <section
      className={styles.modal}
      role="dialog"
      aria-modal="true"
      aria-labelledby="modal-title"
      aria-live="assertive"
      ref={modalRef}
    >
      <div className="visually-hidden">
        모달 창이 열렸습니다. 닫기 버튼 또는 esc 버튼을 눌러 이 창을 닫을 수 있습니다.
      </div>
  • 팝업이 열려있는 동안에는 포커스 이동이 팝업 내에서만 이루어져야 합니다.
  • 스크린 리더로 팝업을 닫을 수 있어야 합니다.
  • 키보드 접근성을 함께 고려합니다. (키보드만으로도 팝업으로 바로 진입해서 팝업을 닫을 수 있어야 합니다)

모달이 열리면 다음을 수행합니다:

  1. 초기 포커스 설정: 이전에 포커스된 요소를 저장한 후, 모달이 열릴 때 "닫기" 버튼에 포커스를 맞춥니다.
  2. 키보드 이벤트 리스너 등록: 사용자가 Esc 키를 눌렀을 때 모달을 닫고, Tab 키를 눌러 포커스를 모달 내에서만 이동할 수 있도록 제한하는 포커스 트랩을 설정합니다.
  3. 포커스 트랩: 사용자가 Tab 또는 Shift + Tab을 눌렀을 때 포커스가 모달 내부에서만 순환하도록 합니다. 포커스가 모달 내부 첫 번째 요소에서 마지막 요소로 또는 마지막 요소에서 첫 번째 요소로 이동하는 것을 방지합니다.
  4. 정리 작업: 모달이 닫히면 이벤트 리스너를 제거하고, 이전에 포커스된 요소로 포커스를 복원합니다.

🧐 우리 팀의 접근성 체크리스트

땅콩의 한 게임을 처음부터 끝까지 하는 기준으로 플로우를 설정했습니다.

땅콩 서비스

방 참여 -> 게임 초대 -> 게임 시작 -> 옵션 선택 -> 라운드 결과

화면을 볼 수 없는 사용자가 이 단계를 완료할 수 있는가?
이 단계에서 제공하는 정보나 지시 사항이 모든 사용자에게 명확한가?

  • 개선 방안

타이머 안내 음성
버튼을 눌렀을 때 해당 옵션을 선택했다는 안내 음성
옵션을 선택하는 버튼임을 알려주는 안내 음성 -> 버튼 마다 옵션1: 내용 옵션2: 내용 을 통해서 개선해보기
게임 페이지에 최초 진입 시 밸런스 게임 질문과 옵션 안내 음성
닉네임 설정 안내 음성
닉네임 최대 글자 수 안내 음성
버튼을 눌렀을 때 해당 옵션을 선택했다는 안내 음성
방이 생성되었는지 안내 음성
카테고리 버튼 포커스 시 방 설정의 모든 정보를 안내 음성

혹시 땅콩의 현 스크린 리더 상황이 더 자세히 알고 싶다면 ! 저의 테크니컬 라이팅에 첨부된 동영상으로도 확인할 수 있어요.
썬데이 테크니컬 라이팅 보러가기
쿠키는 어떤 점들이 개선되면 좋을 것 같나요? 혹시 좋은 의견이 추가로 있다면 알려주세요 🤭🤭

Copy link

@jinhokim98 jinhokim98 left a comment

Choose a reason for hiding this comment

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

안녕하세요 썬데이🌞 저도 썬데이를 리뷰이로 뵙게 되어 반가워요!
벌써 7개월이 지났나요? 시간 정말 빠르네요;; 이제 진짜 수료까지 얼마 안 남았다고 생각하니 아쉬운 마음이 올라오지만 그런 만큼 알차게 보내서 원하는 목표 이룰 수 있길 바랄게요!

우선 주어진 요구 사항대로 잘 개선해주신 것 같아요. 고생하셨습니다.
하지만 중간 중간 궁금한 것도 있고 제가 생각했을 때 의도대로 동작하지 않을 것 같은 내용도 있어서 Request Change 드립니다.

그리고 땅콩 서비스 이용해봤는데 접근성 관련해서 추가로 개선하면 좋을 점은 아래 코멘트에 달게요!

<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>항공권 예약 및 여행 정보 | 썬데이 항공</title>

Choose a reason for hiding this comment

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

본문에 잘 맞는 타이틀을 정하신 것 같아요.
타이틀 만으로도 이 페이지에서 항공권 예약 및 여행 정보를 확인할 수 있겠다는 생각이 드네요👍

src/App.tsx Outdated
Comment on lines 12 to 16
const handleSkipToContent = () => {
if (mainContentRef.current) {
mainContentRef.current.focus();
}
};

Choose a reason for hiding this comment

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

저는 이런 방식으로 해결했어요.

<a href="#main-content" className={styles.skipLink}>
  본문으로 바로가기
</a>

<main id="main-content" className={styles.main}>~~</main>

이렇게 해도 잘 넘어가더라구요! a tag의 href에 원하는 위치의 id를 넣으면 이동하는 것 같아요.

썬데이의 해결방식을 보니 SkipToContent라는 컴포넌트를 만들고 그 안에 handleSkipToContent를 이용해 ref의 focus로 문제를 해결하신 것 같아요. SkipToContent 컴포넌트 내부를 들어가보니 a 태그의 href가 #main-content로 고정되어있고 handleSkipToContent를 받아 실행하는 것을 확인했어요

여기서 몇 가지 의견이 있어서 말씀드리자면

  1. 요구사항의 변화로 skip to content될 대상이 바뀔 때 SkipToContent는 대처가 가능할까?
    만약 요구사항이 바뀌어 main-content가 아닌 footer로 보내야 하는 상황을 생각해봅시다.
    하지만 이미 SkipToContent의 a href 속성값이 main-content로 고정되어있어서 footer로 이동할 수 없을 것 같아요.

  2. handleSkipToContent 대로 skip이 이루어질까?
    이미 a 태그의 href로 main-content로 보내고 있어서 handleSkipToContent 함수가 실행은 되지만 원하는 대로 포커스가 되지 않습니다. 아래는 제가 썬데이의 코드를 수정해서 테스트 해본 결과입니다.

 const footerContent = useRef<HTMLDivElement>(null);

  const handleSkipToContent = () => {
    console.log(footerContent.current);
    console.log('call handleSkipToContent');

    if (footerContent.current) {
      footerContent.current.focus();
    }
  };

 <footer ref={footerContent} className={styles.footer}>
     <p className="body-text">&copy; SUNDAY AIRLINE</p>
  </footer>
2024-10-01.23-34-24.mp4

그래서 제 생각에는 SkipToComponent에 ref를 이용한 조작보다는 a태그의 href prop을 받아 그 위치로 skip하는 기능으로 변경한다면 의도한 대로 작동할 것 같아요😊

Copy link
Author

Choose a reason for hiding this comment

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

쿠키 ! 정말 꼼꼼하시군요. 처음에 skipToContent 컴포넌트를 만들어서 메인 컨텐츠로 보내는 코드를 작성했지만 추가로 ref를 썼던 이유는 focus를 옮기기 위함이었어요. 하지만 의도대로 포커스가 넘어가지 않아서 추가로 포커스를 옮기는 작업을 했고 사실상 필요가 없어진 코드입니다 ..🥹 덕분에 불필요한 코드를 삭제하고 skipToContent를 유연하게 사용할 수 있도록 기본값은 #main-content 이고, linkTaget을 주입받을 수 있도록 코드를 수정했어요 !

import styles from './SkipToContent.module.css';

interface SkipToContentProps {
  linkTarget?: string;
}

const SkipToContent = ({ linkTarget = '#main-content' }: SkipToContentProps) => {
  return (
    <a href={linkTarget} className={styles.skipLink}>
      Skip to content
    </a>
  );
};

export default SkipToContent;

src/App.tsx Outdated

function App() {
const mainContentRef = useRef<HTMLDivElement>(null);

Choose a reason for hiding this comment

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

Suggested change
const mainContentRef = useRef<HTMLDivElement>(null);
const mainContentRef = useRef<HTMLElement>(null);

실제 연결되어있는 ref가 div가 아니라 main이므로 HTMLDivElement보다는 HTMLElement를 쓰는 것이 타입 안정성에 더 좋을 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

꼼꼼한 타입 챙기미 도장 드립니다. 하지만 이 코드는 ref를 삭제하면서 없어졌슴니다 .. 🥹

<div className={styles.flightBooking}>
<FlightBooking />
</div>
<div className={styles.travelSection}>
<h1 className={`${styles.travelTitle} heading-2-text`}>지금 떠나기 좋은 여행</h1>
<h2 className={`${styles.travelTitle} heading-2-text`}>지금 떠나기 좋은 여행</h2>

Choose a reason for hiding this comment

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

h태그의 계층 구조 잘 챙겨주셨어요😊😊

</div>
</main>
<footer className={styles.footer}>
<p className="body-text">&copy; SUNDAY AIRLINE</p>

Choose a reason for hiding this comment

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

나중에 Sunday Airline 진짜 창립하시는건가요?ㅋㅋ

Copy link
Author

Choose a reason for hiding this comment

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

쿠키가 투자 좀 해주세요 ......... 제발 .

<img src={helpIcon} alt="도움말" className={styles.helpIcon} />
{showTooltip && <div className={styles.tooltip}>최대 3명까지 예약할 수 있습니다</div>}
</div>
<img src={helpIcon} alt="" className={styles.helpIcon} />

Choose a reason for hiding this comment

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

아래 MDN 문서를 참고하시면
https://developer.mozilla.org/ko/docs/Web/HTML/Element/img

빈 문자열(alt="")을 사용한 경우, 이미지가 콘텐츠의 중요 부분이 아니므로(장식 또는 추적용 픽셀 등), 비 시각적 브라우저가 렌더링을 하지 않아도 된다는 의미입니다.

라고 나와있는데 스크린 리더의 입장에선 "도움말"을 읽을 필요가 없으니 비워두신 걸까요?

Copy link
Author

Choose a reason for hiding this comment

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

스크린 리더 입장에서는 아래의 음성 안내를 통해 인식할 수 있다고 생각하여 "도움말 이미지"라고 굳이 읽지 않도록 했어요. 하지만 지금 다시 넣어보니 "도움말 + 안내 멘트"가 자연스럽게 이어진다고 판단해서 다시 추가해줬습니다 ! 🤭

border-radius: 4px;
border: none;
text-decoration: none;
transition: top 0.3s ease-in-out;

Choose a reason for hiding this comment

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

이건 성능 최적화 관련된 이야기입니다만

랜더링 성능 개선을 위해 top 대신 transform: translate를 이용하면 좋을 것 같아요

image

Copy link
Author

Choose a reason for hiding this comment

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

오 !! 배운 내용을 또 써보게 하는 쿠키 좋네요 . 바로 반영했습니다 👏👏🤭

<button
className={`${styles.navButton} ${styles.navButtonPrev}`}
onClick={prevTravel}
aria-label="이전 여행 상품"

Choose a reason for hiding this comment

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

이전 여행 상품, 다음 여행 상품 👍👍

Comment on lines 80 to 82
aria-label={`${option.departure}출발${option.destination}도착 ${
option.type
} 가격${option.price.toLocaleString()}원 선택하면 예약 페이지로 이동합니다.`}

Choose a reason for hiding this comment

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

이 부분 스크린 리더가 읽는 속도가 너무 빠른 것 같아요...ㅜ

default.mp4

Copy link
Author

Choose a reason for hiding this comment

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

헉 혹시 쿠키 지금 동영상이 자꾸 이렇게 나와서 .. 확인 후 다시 코멘트 달아도 될까요 ?

2024-10-03.235826.mp4

Choose a reason for hiding this comment

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

영상이 왜 끊기죠? 흠;;;;;;

Copy link
Author

Choose a reason for hiding this comment

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

쿠키 ~ ! 개선했습니다 !! 따로 영상을 보내주셔서 감사해요 🌞👏 중간에 .을 넣어서 끊어 읽도록 했습니다 ! ^. ^

Comment on lines +76 to 79
<button
key={index}
className={`${styles.card} ${index === currentIndex ? styles.cardActive : ''}`}
onClick={() => handleCardClick(option.link)}

Choose a reason for hiding this comment

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

클릭하면 해당 페이지로 이동하는 기능이면 button보다 a태그를 사용하는 것이 더 좋아보입니다😊

Copy link
Author

Choose a reason for hiding this comment

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

맞아요 !! 쿠키 말대로 해당 페이지로 이동하는 기능만 있기 때문에 button보다 a 태그가 더 적합하다고 생각했는데, a 태그로 수정을 하니 하나의 묶음으로 읽어주지 않고 각각 요소에 전부 포커스가 가더라고요. 이런 부분 때문에 키보드 접근성에도 스크린 리더에도 좋지 않다는 생각이 들어 button 태그를 사용하여 구현했습니다 ✨ 혹시 쿠키가 a 태그를 사용하셨으면 어떻게 해결하셨을까요?

Choose a reason for hiding this comment

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

저는 거기까진 신경 쓰지 못 했는데;;; 만약 탭을 주고 싶지 않다면 내부 요소에 tabIndex={-1}을 줄 것 같아요.
그리고 각각 aria-hidden을 주게 되면 스크린 리더가 읽지 않을 것 같아요

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-hidden

Copy link

@jinhokim98 jinhokim98 left a comment

Choose a reason for hiding this comment

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

추가된 내용 리뷰했습니다!


useEffect(() => {
if (isOpen && modalRef.current) {
const previouslyFocusedElement = document.activeElement as HTMLElement;

Choose a reason for hiding this comment

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

이전의 포커스를 갖고 있다가 모달이 언마운트 될 때 원래 갖고 있던 포커스로 되돌린다.
디테일 너무 좋은데요?

Comment on lines +21 to +25
const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === 'Escape') {
closeModal();
}

Choose a reason for hiding this comment

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

esc 눌렀을 때 모달 꺼짐 좋습니다!

}

const focusableElements = modalRef.current?.querySelectorAll<HTMLElement>(
'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'

Choose a reason for hiding this comment

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

요것들은 61번째 라인부터 있는 render 되는 태그들인가요?

Copy link
Author

Choose a reason for hiding this comment

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

넵 !!!!! 포커스가 모달 내에서 이동하기 위함입니다 ✨

Comment on lines +35 to +42
if (document.activeElement === firstFocusableElement) {
lastFocusableElement.focus();
e.preventDefault();
}
} else {
if (document.activeElement === lastFocusableElement) {
firstFocusableElement.focus();
e.preventDefault();

Choose a reason for hiding this comment

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

이 부분은 모달 내부 태그에서 마지막 태그에 도달했을 경우에 탭을 누르면 모달의 첫 번째 요소로 되돌아가도록 하신거죠?

이건 정말 몰라서 물어보는 이야기인데 Shift + Tab을 누르면 동작이 달라지나요?

Copy link
Author

Choose a reason for hiding this comment

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

네 shift + tab 사용 시 이전 요소로 포커스가 이동합니다 ✨

Choose a reason for hiding this comment

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

오~ 덕분에 하나 알아갑니다😊

return (
<section
className={styles.modal}
role="dialog"

Choose a reason for hiding this comment

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

role="dialog" 좋아요!

role="dialog"
aria-modal="true"
aria-labelledby="modal-title"
aria-live="assertive"

Choose a reason for hiding this comment

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

aria-live 옵션 중 "assertive"로 설정하신 이유가 궁금합니다!

Copy link
Author

Choose a reason for hiding this comment

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

이렇게 설정을 하지 않으니 모달 창이 열렸다는 안내보다 모달의 내용을 먼저 읽고 마지막에 모달 창이 열렸다는 안내를 해주더라고요 ! 🤭

Choose a reason for hiding this comment

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

아 먼저 모달 창이 열렸다고 말해주기 위함이었군요!

@jinhokim98
Copy link

땅콩 접근성 개선됐으면 좋을 점 드릴게요

  1. 방 만든 후 닉네임 설정 autofocus로 인해 스크린 리더가 닉네임을 설정해야 한다는 것을 읽어주면 좋을 것 같아요
default.mp4
  1. 모달이 켜질 때 포커스가 뒤로 유지되어있어요
default.mp4
  1. 스크린리더로 의지해야 하는 사용자에겐 고를 수 있는 시간이 너무 짧은 것 같아요. 내용을 다 읽었는데 남은 시간은 1초였어요.
default.mp4
  1. 사소하지만 왕관 아이콘을 방장이라고 읽어도 좋을 것 같아요

왕관 아이콘

Copy link
Author

@useon useon left a comment

Choose a reason for hiding this comment

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

쿠키 !! 🍪🍪 꼼꼼한 리뷰 너무 감사드려요 .. ! 칭찬도 많이 해주셔서 기분이 좋았습니다요 호호✨🤭 땅콩 접근성 문제도 플레이 하면서 하나하나 말씀해 주셔서 큰 도움이 될 것 같아요 ! 감동 ,, 🥹 서프라이즈 사전 미션 등장에 코멘트 반영이 살짝 늦어졌는데 이해해 주셔서 감사해요 👏👏 질문에 대한 답변과 리뷰 반영 찬찬히 확인해 주시고, 궁금한 부분 있으면 편하게 달아주세요! SSR 미션도 프로젝트도 파이팅입니다 쿠키 !!!! 🌞

src/App.tsx Outdated
Comment on lines 12 to 16
const handleSkipToContent = () => {
if (mainContentRef.current) {
mainContentRef.current.focus();
}
};
Copy link
Author

Choose a reason for hiding this comment

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

쿠키 ! 정말 꼼꼼하시군요. 처음에 skipToContent 컴포넌트를 만들어서 메인 컨텐츠로 보내는 코드를 작성했지만 추가로 ref를 썼던 이유는 focus를 옮기기 위함이었어요. 하지만 의도대로 포커스가 넘어가지 않아서 추가로 포커스를 옮기는 작업을 했고 사실상 필요가 없어진 코드입니다 ..🥹 덕분에 불필요한 코드를 삭제하고 skipToContent를 유연하게 사용할 수 있도록 기본값은 #main-content 이고, linkTaget을 주입받을 수 있도록 코드를 수정했어요 !

import styles from './SkipToContent.module.css';

interface SkipToContentProps {
  linkTarget?: string;
}

const SkipToContent = ({ linkTarget = '#main-content' }: SkipToContentProps) => {
  return (
    <a href={linkTarget} className={styles.skipLink}>
      Skip to content
    </a>
  );
};

export default SkipToContent;

src/App.tsx Outdated

function App() {
const mainContentRef = useRef<HTMLDivElement>(null);
Copy link
Author

Choose a reason for hiding this comment

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

꼼꼼한 타입 챙기미 도장 드립니다. 하지만 이 코드는 ref를 삭제하면서 없어졌슴니다 .. 🥹

</div>
</main>
<footer className={styles.footer}>
<p className="body-text">&copy; SUNDAY AIRLINE</p>
Copy link
Author

Choose a reason for hiding this comment

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

쿠키가 투자 좀 해주세요 ......... 제발 .

Comment on lines +17 to +23
const updateAlert = (message: string) => {
setStatusMessage('');
setTimeout(() => {
setStatusMessage(message);
}, 0);
};

Copy link
Author

Choose a reason for hiding this comment

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

이 코드를 작성한 이유는 이미 최소 승객 수/최대 승객 수가 도달한 이후에 다시 -나 +버튼을 눌렀을 때 음성 안내를 하지 않더라고요. 저는 최소/최대 승객 수 도달한 상태에서 그 버튼을 누를 때마다 안내를 해 주고 싶어서 해당 코드를 작성했습니다. 메세지를 다시 초기화하고 다시 담는 방식으로 .. ! 🤭

<div className={styles.flightBooking}>
<h2 className="heading-2-text">항공권 예매</h2>
<article className={styles.flightBooking}>
<h2 className="heading-2-text" tabIndex={0}>
Copy link
Author

Choose a reason for hiding this comment

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

tabIndex={-1} 을 주면 키보드 포커스가 원래 가는 요소일지라도 건너뜁니다.
tabIndex={1} 은 tabIndex={0} 보다 높은 우선 순위를 가집니다.

하지만 0보다 큰 양수 값 사용은 피하라고 되어 있네요 ! 👏👏

참고: https://developer.mozilla.org/ko/docs/Web/HTML/Global_attributes/tabindex

role="dialog"
aria-modal="true"
aria-labelledby="modal-title"
aria-live="assertive"
Copy link
Author

Choose a reason for hiding this comment

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

이렇게 설정을 하지 않으니 모달 창이 열렸다는 안내보다 모달의 내용을 먼저 읽고 마지막에 모달 창이 열렸다는 안내를 해주더라고요 ! 🤭

}

const focusableElements = modalRef.current?.querySelectorAll<HTMLElement>(
'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
Copy link
Author

Choose a reason for hiding this comment

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

넵 !!!!! 포커스가 모달 내에서 이동하기 위함입니다 ✨

Comment on lines 80 to 82
aria-label={`${option.departure}출발${option.destination}도착 ${
option.type
} 가격${option.price.toLocaleString()}원 선택하면 예약 페이지로 이동합니다.`}
Copy link
Author

Choose a reason for hiding this comment

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

헉 혹시 쿠키 지금 동영상이 자꾸 이렇게 나와서 .. 확인 후 다시 코멘트 달아도 될까요 ?

2024-10-03.235826.mp4

border-radius: 4px;
border: none;
text-decoration: none;
transition: top 0.3s ease-in-out;
Copy link
Author

Choose a reason for hiding this comment

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

오 !! 배운 내용을 또 써보게 하는 쿠키 좋네요 . 바로 반영했습니다 👏👏🤭

Comment on lines +76 to 79
<button
key={index}
className={`${styles.card} ${index === currentIndex ? styles.cardActive : ''}`}
onClick={() => handleCardClick(option.link)}
Copy link
Author

Choose a reason for hiding this comment

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

맞아요 !! 쿠키 말대로 해당 페이지로 이동하는 기능만 있기 때문에 button보다 a 태그가 더 적합하다고 생각했는데, a 태그로 수정을 하니 하나의 묶음으로 읽어주지 않고 각각 요소에 전부 포커스가 가더라고요. 이런 부분 때문에 키보드 접근성에도 스크린 리더에도 좋지 않다는 생각이 들어 button 태그를 사용하여 구현했습니다 ✨ 혹시 쿠키가 a 태그를 사용하셨으면 어떻게 해결하셨을까요?

@jinhokim98 jinhokim98 self-assigned this Oct 4, 2024
Copy link

@jinhokim98 jinhokim98 left a comment

Choose a reason for hiding this comment

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

안녕하세요 썬데이🌞 접근성 미션 고생 많으셨습니다.
코드를 읽으며 놀란 부분이 한 두 곳이 아닐 정도로 깊이 고민하신 모습이 보였습니다. 이번 미션을 통해 썬데이가 많은 것을 얻어간 것 같아 뿌듯하네요! 리뷰도 잘 반영해주셔서 충분한 것 같아 이만 머지 하겠습니다!!
남은 기간 동안 열심히 학습하고 경험해서 썬데이만의 알맹이를 가져가셨으면 합니다!
남은 미션도 파이팅입니다🚀🚀

@jinhokim98 jinhokim98 merged commit af81559 into woowacourse:useon Oct 4, 2024
@useon useon assigned useon and unassigned jinhokim98 Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants