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

Migrate course and project related blocks #50

Merged
merged 11 commits into from
Mar 25, 2024

Conversation

yumincho
Copy link
Contributor

Description

@/components/blocks 중 4개 block을 .tsx로 전환하면서 다음 작업을 진행했습니다.

  • prop-types을 제거하고 interface를 도입했습니다.
  • type annotation이 필요한 경우 추가했습니다.
  • i18n 중 일부를 useTranslatedString hook을 사용하는 것으로 변경했습니다.
  • docstring을 추가했습니다.

Checklist

  • CourseBlock.tsx
  • CourseSimpleBlock.tsx
  • PlannerCourseBlock.tsx
  • ProjectBlock.tsx

@yumincho yumincho changed the base branch from main to migration March 17, 2024 11:48
@yumincho yumincho self-assigned this Mar 17, 2024
Copy link

PR Preview Action v1.4.7
🚀 Deployed preview to https://sparcs-kaist.github.io/otlplus-web/pr-preview/pr-50/
on branch gh-pages at 2024-03-17 11:48 UTC

Copy link

[PR Preview GH Proxy (on bap)]
👍 Proxied Github Pages Preview URL to http://pr-50.otl-pr-preview.sparcsandbox.com/index.html
Proudly Powered by [email protected]

@yumincho yumincho marked this pull request as ready for review March 17, 2024 11:49
@useruseruse
Copy link
Contributor

먼저 PR 올려주셔서 작업하는데 참고하고 있습니다! 혹시 custom translation hook 이 필요한 부분이 아닌 다른 부분에서 useTranslation hook 이 아니라 withTranslation HOC 을 사용한 이유가 있는지 궁금합니다!

@yumincho yumincho added the migrate migrate from JS CC to TS FC label Mar 19, 2024
);
};

export default withTranslation()(React.memo(CourseSimpleBlock));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@useruseruse 이 부분을 말씀하신 거라면 원래부터 withTranslation()을 사용하고 있어서 변경하지 않았습니다. 다른 부분을 말씀하신 거라면 혹시 인용해서 좀 더 자세히 설명해주실 수 있나요?

}

/**
* Component `CourseSimpleBlock` displays a brief overview of a course within the `CourseRelatedCoursesSubSection` on the `DictionaryPage`.
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

@sboh1214 sboh1214 left a comment

Choose a reason for hiding this comment

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

몇가지 제안 해보았습니다~ :)

@sboh1214 sboh1214 force-pushed the migration@components/blocks3 branch from 00fd48e to 76ae674 Compare March 25, 2024 16:01
Copy link
Contributor

@sboh1214 sboh1214 left a comment

Choose a reason for hiding this comment

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

LGTM

@sboh1214 sboh1214 merged commit e0f6262 into migration Mar 25, 2024
3 checks passed
@sboh1214 sboh1214 deleted the migration@components/blocks3 branch March 25, 2024 16:03
@yumincho yumincho added this to the TypeScript Migration milestone Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migrate migrate from JS CC to TS FC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants