-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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(ion-datetime): add hidden-days when theme:ionic #30205
base: next
Are you sure you want to change the base?
Conversation
… is true; - change datetime component to use this attribute when in theme: ionic; - document changes;
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add more comments of what the code is doing. Datetime is already complex, this new logic should be documented well so the devs aren't spending too much time trying to figure out what's going on.
days = [{ day: null, dayOfWeek: null }, ...days]; | ||
} else { | ||
for (let i = 0; i <= offset; i++) { | ||
days = [{ day: null, dayOfWeek: null, hiddenDay:true }, ...days]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't hiddenDay
be false?
if(hiddenDay && day !== null && day > 20) { | ||
// Leading with the hidden day from the previous month | ||
// if its a hidden day and is higher than '20' (last week even in feb) | ||
if(month === 1) { | ||
_year = year - 1; | ||
_month = 12; | ||
}else{ | ||
_month = month-1; | ||
} | ||
} else if(hiddenDay && day !== null && day < 15) { | ||
// Leading with the hidden day from the next month | ||
// if its a hidden day and is lower than '15' (first two weeks) | ||
if(month === 12) { | ||
_year = year + 1; | ||
_month = 1; | ||
} else { | ||
_month = month + 1; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this calculation be done through getDaysOfMonth
?
Issue number: internal
What is the current behavior?
What is the new behavior?
Does this introduce a breaking change?
Other information