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

Master #3

Open
wants to merge 6 commits into
base: hw2
Choose a base branch
from
Open

Master #3

wants to merge 6 commits into from

Conversation

Voravomas
Copy link
Owner

rewrote project using classes

@Voravomas Voravomas requested a review from pavlo-berezin June 2, 2020 12:03
@pavlo-berezin pavlo-berezin changed the base branch from hw2 to hw1 June 10, 2020 21:53
@pavlo-berezin pavlo-berezin changed the base branch from hw1 to hw2 June 10, 2020 21:53

export default class CalendarDay extends React.Component {
render() {
let curDay = new Date(this.props.info.year, this.props.info.month, this.props.info.day);
Copy link
Collaborator

Choose a reason for hiding this comment

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

move info to own variable, you using this.props.info too much.

</Header>
<Sidebar>
<button className='btn' onClick={() => this.props.chgDay(-1)}>Prev day</button>
<button className='btn' onClick={() => this.props.chgDay(-2)}>Next day</button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

why next day is -2?

if (numOfDaysFromBegOfWeek !== 6) {
for (let j = 1; j <= numOfDaysFromBegOfWeek + 1; j++) {
arr.push({
date: (-1) * j,
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not date: -j?


}

changeMonth = (what) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this function is totally counter-intuitive, please refactor this. :)

}
}

isOkDay = (day) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can reduce size of this function if you investigate working with Date objects more :)

@pavlo-berezin pavlo-berezin self-requested a review June 11, 2020 21:49
@pavlo-berezin pavlo-berezin removed their request for review December 20, 2021 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants