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

feat: add option to start on Monday (#18) #70

Merged
merged 5 commits into from
Mar 18, 2021

Conversation

lja1018
Copy link
Contributor

@lja1018 lja1018 commented Mar 4, 2021

  • 한 주의 시작을 일요일이 아닌 요일로 지정할 수 있는 옵션 weekStartDay을 추가하였습니다.

    • 'Sun'(default), 'Mon', 'Tue', ..., 'Sat'
  • AS-IS (일요일 시작 고정)

    • 스크린샷 2021-03-04 오후 4 45 14
  • TO-BE (월요일 시작 (weekStartDay: 'Mon'))

    • 스크린샷 2021-03-04 오후 4 45 56
  • TO-BE (화요일 시작 (weekStartDay: 'Tue'))

    • 스크린샷 2021-03-08 오후 4 45 35
  • etc..

@jung-han
Copy link
Member

jung-han commented Mar 5, 2021

메신저에서 말씀드린대로 월요일에 한정짓기 보다는 인덱스를 받아서 처리하는게 나을 수도 있을 것 같습니다.

다른 라이브러리 참고 부탁드립니다!

src/js/calendar/index.js Outdated Show resolved Hide resolved
Copy link
Member

@jung-han jung-han left a comment

Choose a reason for hiding this comment

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

동작 안되는 부분이랑 예제 코드, 테스트 추가 부탁드릴게요

src/js/calendar/body.js Outdated Show resolved Hide resolved
test/calendar/body.spec.js Outdated Show resolved Hide resolved
test/calendar/body.spec.js Outdated Show resolved Hide resolved
Copy link
Member

@seonim-ryu seonim-ryu left a comment

Choose a reason for hiding this comment

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

리뷰 완료합니다~ for문이 3번 연속으로 실행되는 함수가 있는데 속도도 한 번 체크해주시면 좋을 것 같습니다! 고생하셨어요~

Copy link
Member

@shiren shiren left a comment

Choose a reason for hiding this comment

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

수고하셨어요.
수정되면 다시 볼게요

src/js/calendar/index.js Outdated Show resolved Hide resolved
Copy link
Member

@jung-han jung-han left a comment

Choose a reason for hiding this comment

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

완료입니다. 다른 분들이 이미 리뷰를 다 해주셨네요. 고생하셨습니다~

@lja1018 lja1018 requested a review from heenakwag March 16, 2021 07:06
Copy link

@dongsik-yoo dongsik-yoo left a comment

Choose a reason for hiding this comment

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

리뷰 완료합니다. 수고하셨습니다.

src/js/calendar/layerBody/date.js Outdated Show resolved Hide resolved
src/js/dateRangePicker/index.js Outdated Show resolved Hide resolved
Copy link
Member

@seonim-ryu seonim-ryu left a comment

Choose a reason for hiding this comment

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

리뷰 완료합니다. 이건 팁인데 처음 리뷰를 받고 추가 작업을 하는 경우 별도 PR로 분리하거나 특정 해시 커밋으로 리뷰를 요청하면 리뷰를 더 쾌적하게 할 수 있습니다ㅎㅎ 고생하셨어요!

Copy link
Member

@jung-han jung-han 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
Member

@heenakwag heenakwag left a comment

Choose a reason for hiding this comment

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

[3/17] 리뷰 완료했습니다.

src/js/calendar/index.js Show resolved Hide resolved
src/js/dateRangePicker/index.js Outdated Show resolved Hide resolved
@lja1018 lja1018 merged commit dddb9d2 into master Mar 18, 2021
@lja1018 lja1018 deleted the chore/add-mon-start-option branch March 18, 2021 03:15
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.

7 participants