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

프로그레스 페이지 반응형 디자인 구현 및 접근성 디자인 적용 #188

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

SamTheKorean
Copy link
Contributor

@SamTheKorean SamTheKorean commented Jan 5, 2025

체크리스트

  • 이슈가 연결되어 있나요?
  • 배포 후 브라우저 콘솔에 경고나 오류가 있나요?

구현 사진

#모바일 & 테블릿
Screenshot 2025-01-09 at 12 49 08 AM
Screenshot 2025-01-09 at 12 49 19 AM

#데스크톱

Screenshot 2025-01-09 at 12 50 28 AM

@SamTheKorean SamTheKorean self-assigned this Jan 5, 2025
@SamTheKorean SamTheKorean requested a review from a team as a code owner January 5, 2025 17:28
@SamTheKorean SamTheKorean linked an issue Jan 5, 2025 that may be closed by this pull request
@SamTheKorean SamTheKorean marked this pull request as draft January 6, 2025 03:35
@SamTheKorean
Copy link
Contributor Author

ui 변경 사항 리뷰 중 수정사항이 보여서 draft로 전환했습니다.

@SamTheKorean
Copy link
Contributor Author

변경필요한 부분 수정하고 커밋한 뒤에 다시 pr 열었습니다!

/>
);
}
const iconClass = completed ? styles.completedIcon : styles.incompleteIcon;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit) 다른 부분도 모두 삼항연사자로 작성하셨는데 이 부분만 따로 변수로 지정한 이유가 궁금합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

다른 부분과 일관되게 바꿔보겠습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분 수정해주신다고 하셨는데 수정됐을까요~?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

피드백을 지금 완전히 이해했습니다!

function getTaskIcon(completed: boolean) {
  return (
    <img
      src={`/ ${completed ? "completed" : "incomplete"}-status-icon.svg`}
      alt={`${completed ? "완료" : "미완료"} 문제 상태 아이콘`}
      aria-label={`${completed ? "완료" : "미완료"} 문제`}
      className={completed ? styles.completedIcon : styles.incompleteIcon}
    />
  );
}
```  이렇게 class부분까지 삼항 연산자로 옮기면 src가 null로 처리되고 정상적으로 렌더링 되지 않고 alt 값이 렌더링되는 현상이 발생해서 변수를 사용했는데 이유를 분석중에 있습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

확인해보니 /뒤에 공백이 있어서 문제가 발생했던 것 같네요! 해결하고 push했습니다! 피드백 감사합니다!

Comment on lines 2 to 5
display: flex;
z-index: var(--z-index-aside);
max-width: 203px;
Copy link
Contributor

Choose a reason for hiding this comment

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

position이 static일땐 z-index는 필요없다고 생각해요.
media query에 적용하면 어떨까요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그렇게 하는게 좋겠네요! 피드백 감사합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

피드백 반영하고 push 했습니다 감사합니다! @Sunjae95

@DaleSeo DaleSeo linked an issue Jan 12, 2025 that may be closed by this pull request
@SamTheKorean
Copy link
Contributor Author

메모 : 병합 후 접근성 테스트 활성화 해야함

@SamTheKorean SamTheKorean merged commit c638f6a into main Jan 13, 2025
6 checks passed
@SamTheKorean SamTheKorean deleted the 171 branch January 13, 2025 23:18
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.

색깔 접근성 문제 해결 구현 Responsive Implmentation for Progress
2 participants