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] Refactor/#628 시맨틱 태그, 웹 표준, 웹 접근성 검토 #629

Merged
merged 30 commits into from
Dec 29, 2023

Conversation

semnil5202
Copy link
Collaborator

@semnil5202 semnil5202 commented Nov 22, 2023

작업 대상

  • 서비스 전체 태그를 대상으로 시맨틱 태그 작업
  • 서비스 전체 플로우를 대상으로 접근성 로직 유지보수
  • PinPreview 체크 박스와 친구추가 모달 체크 박스 UI 통일화

📄 작업 내용

시맨틱 태그

  • 커밋 로그에서도 볼 수 있듯이 시맨틱 태그를 보완했습니다. article과 section 경계면이 모호하여 topicCard와 같이 독단적으로 쓰일 수 있는 컴포넌트는 모두 article 태그로 지정하고, 영역을 나타내는 곳을 section으로 지정하였습니다.
  • 머리말 (예를 들면, 인기 급상승할 지도? 새로울 지도? 등) 영역은 h2, h3 태그를 사용하여 보다 페이지를 대표하는 머리말 역할을 할 수 있도록 하였습니다.

웹 접근성

  • 서비스 전반 플로우에 대하여 aria-label, role, tabIndex, aria-hidden 등을 사용하여 접근성 로직을 유지보수 하였습니다.
  • SVG 아이콘이자 버튼의 기능을 하는 것들 대부분이 포커스가 누락되어 있어서 이를 보완하였습니다.
  • 지도 추가, 핀 추가 페이지에 접근성 로직이 누락되어 이를 보완하였습니다. 핀 추가 페이지의 경우 핀이란 도메인 단어 대신 장소로 일괄 수정하였습니다.
  • 핀 상세보기 페이지에 접근성 로직이 누락되어 이를 보완하였습니다.
  • 장소 댓글 부분에서 접근성 로직이 누락되어 이를 보완하였습니다.

웹 표준

  • 크로스 브라우징 이슈는 딱히 없었습니다만, 사파리 브라우저에서 checkbox UI가 크게 달라지는 부분이 있어서 이를 @아이크 가 이전에 작업했던 친구들에게 권한 추가 모달의 checkbox UI를 가져와 적용하였습니다. 체크 박스 색이 primary 로 되어 있던 것을 checked 라는 칼라로 새로 지정하였습니다.

🙋🏻 주의 사항

  • 개발 서버 API가 없어졌으므로 운영 서버 API로 변경하였습니다. 테스트 시 이 점 유의해주세요.
  • 접근성 로직에서 PinPreview를 클릭하여 PinDetail 페이지로 넘어가는게 자연스러운 플로우지만 포커스가 순차 적용되어있기에 이 부분이 미흡합니다.

스크린샷

📎 관련 이슈

close #628

레퍼런스

@semnil5202 semnil5202 added FE 프론트엔드 관련 이슈 우선순위 : 상 refactor 리팩토링 관련 이슈 labels Nov 22, 2023
@semnil5202 semnil5202 self-assigned this Nov 22, 2023
@cpot5620
Copy link
Collaborator

cpot5620 commented Dec 3, 2023

in progress..

@semnil5202 semnil5202 marked this pull request as ready for review December 3, 2023 15:20
Copy link
Collaborator

@GC-Park GC-Park left a comment

Choose a reason for hiding this comment

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

아따 웹 접근성 이거 다 하기 힘들었을텐데 너무 고생했습니다 세인!! 궁금한게 많지는 않아서 바로 approve 할게용 ~!!

Comment on lines +53 to +54
tabIndex={ARIA_FOCUS}
role="button"
Copy link
Collaborator

Choose a reason for hiding this comment

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

스크린리더에 이로운 role까지 주다니 굿 👍

Comment on lines +16 to +17

export const ARIA_FOCUS = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

오.. 그 tabIndex를 하나로 통일해도 순서대로 잘 가던가요 ?!!! 저는 안 갔어서 각자 우선순위에 맞게 값을 줬었는데!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네 저도 안되는 걸로 인지했었는데 목표한 탭 포커스가 모두 0으로 순차적이라면 가능하더군요. MDN에 의하면 대화형(input 등) 콘텐츠는 tabIndex의 값이 0이 기본값이고, 비 대화형은 -1이 기본값이라고 하네요. 원래 저희가 목표한 포커스 순서는 보통 input -> input -> textarea -> button 이와 같고 이를 위해 1, 2, 3, 4, 등등 양수값을 지정해주었었죠.

하지만 0, 0, 0, 0 으로 순서만 맞다면 DOM 순서대로 차례로 포커스가 먹고 이는 1, 2, 3, 4 를 지정한것과 같은 효과를 보았습니다. 이 원리에 대해서는 좀 더 자세히 알아볼 필요가있을거 같은데 혹시 알고 계신내용이 있다면 공유 부탁드립니답~

<MoreReplyButton onClick={toggleSeeMore}>
<MoreReplyButton
onClick={toggleSeeMore}
aria-label={seeMore ? '답글 숨기기' : '답글 보기'}
Copy link
Collaborator

Choose a reason for hiding this comment

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

오호 역할이 바뀔 때도 aria-label이 구분되게 해주셨군요!!! 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

오호 실시간 갱신 정보를 읽기 위한 custom hook이라 좋은데요? 처음엔 세인의 강의가 필요했었는데 사용처를 확인하니 이해할 수 있었습니다! 굿!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

요즘 추상화에 대해서 생각이 많아지더군요.. 저희 코드 리팩토링 할 때 토스 진유림님의 발표를 참고하면 좋지 않을까싶습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

접근성 지저분하게 있었는데 깔끔해지네여

@@ -90,6 +90,7 @@ function TopicInfo({

return (
<Flex
as="article"
Copy link
Collaborator

Choose a reason for hiding this comment

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

as라는 속성을 처음보네요!! 와우 찾아보긴 했는데 웹 성능 최적화를 위하여 추가해주신 걸까요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아뇽 이건 시맨틱 태그를 위한 작업입니다. 원래 const Component = styled.div; 이런식으로 선언한 스타일드 컴포넌트라고 해도 <Component as="artcle" ... /> 로 as prop 을 지정할 경우 const Component = styled.article; 로 선언한 것과 같은 효과를 봅니다.

https://styled-components.com/docs/basics#extending-styles

Copy link
Collaborator

@jiwonh423 jiwonh423 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
Collaborator

Choose a reason for hiding this comment

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

접근성 지저분하게 있었는데 깔끔해지네여

@semnil5202 semnil5202 merged commit 4e3a419 into develop-FE Dec 29, 2023
1 check passed
@semnil5202 semnil5202 deleted the refactor/#628-semantic branch December 29, 2023 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 관련 이슈 refactor 리팩토링 관련 이슈 우선순위 : 상
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants