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

수료증 페이지 구현 #89

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

Sunjae95
Copy link
Contributor

@Sunjae95 Sunjae95 commented Nov 23, 2024

체크리스트

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

수료증 페이지 전체 구현합니다.
데모를 위해 style부분이 미흡하여 다음 스프린트에서 헬레나님과 소통후에 수정하겠습니다.

Shot 2024-11-23 at 17 53 55

@Sunjae95 Sunjae95 self-assigned this Nov 23, 2024
@Sunjae95 Sunjae95 linked an issue Nov 23, 2024 that may be closed by this pull request
@Sunjae95 Sunjae95 marked this pull request as ready for review November 23, 2024 22:08
@Sunjae95 Sunjae95 requested a review from a team as a code owner November 23, 2024 22:08
@DaleSeo
Copy link
Contributor

DaleSeo commented Nov 23, 2024

데모를 위해 style부분이 미흡하여 다음 스프린트에서 헬레나님과 소통후에 수정하겠습니다.

이 부분은 따로 티켓을 생성하실 건가요. 이 PR이 병합되어도 현재 티켓을 오픈 상태로 유지하실 건가요?

@DaleSeo
Copy link
Contributor

DaleSeo commented Nov 23, 2024

@Sunjae95 PR 설명에 디스코드에 올려주신 pdf 캡쳐해서 추가해뒀어용~

@DaleSeo
Copy link
Contributor

DaleSeo commented Nov 23, 2024

스토리가 반쪽 짜리라서 좀 많이 아쉬운 것 같아요 😢 이 스토리북 애드온을 써보시면 어떨까요? https://github.com/storybookjs/addon-queryparams

Shot 2024-11-23 at 18 01 22

Comment on lines 193 to 195
<h3>CERTIFICATE OF ACHIEVEMENT</h3>
<h4>DaleStudy</h4>
<h5>{member?.id}</h5>
Copy link
Contributor

Choose a reason for hiding this comment

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

<h1> 다음에 <h2> 건너뛰고 바로 <h3>가 나오면 안 됩니다. (참고: https://dequeuniversity.com/rules/axe/4.10/heading-order)


<p>{`For successfully completing ${member?.solvedProblems.length === 75 ? "all" : member?.solvedProblems.length} problems\nin the LeetCode Blind 75 and contributing\nto knowledge sharing in the ${member?.cohort}${cohortSuffix?.[member?.cohort ?? 0] ?? "th"} DaleStudy.`}</p>

<img src={Signature} />
Copy link
Contributor

Choose a reason for hiding this comment

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

이미지 요소에는 꼭 alt 속성을 달아주세요.

@Sunjae95
Copy link
Contributor Author

데모를 위해 style부분이 미흡하여 다음 스프린트에서 헬레나님과 소통후에 수정하겠습니다.

이 부분은 따로 티켓을 생성하실 건가요. 이 PR이 병합되어도 현재 티켓을 오픈 상태로 유지하실 건가요?

현재 PR에서 리뷰반영과 작업하겠습니다!

@yolophg
Copy link

yolophg commented Nov 28, 2024

데모를 위해 style부분이 미흡하여 다음 스프린트에서 헬레나님과 소통후에 수정하겠습니다.

이 부분은 따로 티켓을 생성하실 건가요. 이 PR이 병합되어도 현재 티켓을 오픈 상태로 유지하실 건가요?

현재 PR에서 리뷰반영과 작업하겠습니다!

저랑 소통하겠다는 부분이 방금 피그마에 달아주신 간격 관련한 부분일까요? 아니면 또 다른 부분이 있으신걸까요?
추가적으로 있으시면 피그마에서 최대한 비동기적으로 원활히 소통할 수 있도록 달아주시면 감사드리겠습니다!

@Sunjae95
Copy link
Contributor Author

Sunjae95 commented Nov 28, 2024

@yolophg 맞습니다. 피그마 코멘트 확인했는데 제가 원하는 답이였어요 답변 감사합니다!

Comment on lines +6 to +10
parameters: {
query: {
member: "daleseo",
},
},
Copy link
Contributor

Choose a reason for hiding this comment

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

🏅

@@ -3,6 +3,11 @@ import Certificate from "./Certificate";

const meta: Meta<typeof Certificate> = {
component: Certificate,
parameters: {
query: {
member: "daleseo",
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

Choose a reason for hiding this comment

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

이 파일에서 발생하는 충돌을 해결해주시기 바랍니다. 바이너리 파일이라서 쪼끔 주의가 필요한데요.
선재님이 파일에 가하신 변경을 원복하고 bun add 명령어로 @storybook/addon-queryparams를 다시 설치하시면 됩니다.

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.

Entire Certificate Page
3 participants