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

공통 프로필 및 토글 프로필 컴포넌트 제작 #40

Merged
merged 13 commits into from
Nov 21, 2020
Merged

Conversation

dididy
Copy link
Member

@dididy dididy commented Nov 5, 2020

resolve #33

  • 공통 프로필 이미지 컴포넌트
  • 토글바 Image View
  • 참여도 Image View

해결해야하는 추가 문제

YONGJAE LEE and others added 4 commits November 6, 2020 02:00
test 코드에 png 이미지가 있을 경우 테스트가 안되는 이슈 발생
- [ ] 아이콘 컴포넌트 width, height 조절이 필요함
AppleSDGothicNeoEM00 to AppleSDGothicNeoM00
dididy pushed a commit that referenced this pull request Nov 12, 2020
@dididy dididy changed the title Image View 컴포넌트 제작 공통 프로필 및 토글 프로필 컴포넌트 제작 Nov 12, 2020
Copy link
Contributor

@Lavegaa Lavegaa left a comment

Choose a reason for hiding this comment

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

아이콘 수정해드리겠습니다~

@dididy dididy self-assigned this Nov 15, 2020
@dididy dididy added the 🏁 feature 기능 개발 label Nov 16, 2020
Lavegaa and others added 5 commits November 20, 2020 02:25
- rebase를 이런식으로 하는게 맞는지 확실하지 않음(cherry-pick 사용)
  - 찬호님이 이슈 해결한 커밋 땡겨와서 적용 후 작업했음
- 프로필 이미지 경로 변경
@dididy
Copy link
Member Author

dididy commented Nov 19, 2020

@Lavegaa @SkynI25 rebase를 이렇게 하는 것이 맞나요? 원하는 커밋만 가져오기 위해서 cherry-pick을 했는데 커밋id가 변경된 모양이네요..

코드리뷰 부탁드립니다~

@Lavegaa
Copy link
Contributor

Lavegaa commented Nov 20, 2020

음... cherry pick commit 을 해본적이 없어서 잘 모르겠네요..ㅠㅠ 항상 master에 rebase해서 사용했습니다ㅠㅠ

--수정

cherry pick으로 제 #51의 commit부분을 당겨서 pull하신건가요?
그렇다면 전체를 당겨오는 rebase와는 또다른 장점이 있겠네요!

Copy link
Contributor

@Lavegaa Lavegaa left a comment

Choose a reason for hiding this comment

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

전체적으로 깔끔하게 잘 만드신것 같습니다. 간단한 수정 후 머지하시면 될 것 같습니다!


ProfileIcon.propTypes = {
src: PropTypes.string,
size: PropTypes.number,
Copy link
Contributor

Choose a reason for hiding this comment

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

size가 small로 들어오는데 숫자로 정의됐습니다. string으로 수정하면 좋을것같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Lavegaa boolean으로 구조를 변경해서 반영하였습니다!


const profileImage = 'https://avatars2.githubusercontent.com/u/16266103?s=460&u=46ab2774d38212f0d0050592ce02dbcf36a7a97a&v=4';

function renderProfileInfoContainer(name='홍길동', participateRate=90, src=profileImage) {
Copy link
Contributor

Choose a reason for hiding this comment

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

띄어쓰기가 lint에 잡혔습니다.

function renderProfileInfoContainer(name = '홍길동', participateRate = 90, src = profileImage) {

다들 번거롭지만 소중한 lint화이팅 합시다...하나씩 잡고가기가 쉽지가 않네요ㅠㅠ

Copy link
Member Author

Choose a reason for hiding this comment

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

@Lavegaa 반영하였습니다!

- ProfileInfoContainer.test.js에 띄어쓰기 제거
- 기존에 문자열을 넘기는 방식에서 isSmall이라는 boolean 값을 넘겨주도록 변경
- 연관된 컴포넌트 코드들에 반영
Copy link
Contributor

@SkynI25 SkynI25 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 ㅎㅎ 저도 rebase를 할땐 항상 master 에 하도록 했는데 필요한 커밋만 가져와서 하는 방식도 있군요. 새롭게 배웁니다!

@dididy dididy merged commit d506b3c into master Nov 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏁 feature 기능 개발
Projects
None yet
Development

Successfully merging this pull request may close these issues.

공통 프로필 및 토글 프로필 컴포넌트 제작 컴포넌트 제작
3 participants