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(ion-datetime): add hidden-days when theme:ionic #30205

Draft
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

JoaoFerreira-FrontEnd
Copy link

Issue number: internal


What is the current behavior?

What is the new behavior?

  • add logic to generate more days per month if the displayHiddenDays flag is true;
  • change datetime component to use this attribute when in theme: ionic;
  • document changes;

Does this introduce a breaking change?

  • Yes
  • No

Other information

… is true;

- change datetime component to use this attribute when in theme: ionic;
- document changes;
@JoaoFerreira-FrontEnd JoaoFerreira-FrontEnd added type: feature request a new feature, enhancement, or improvement package: core @ionic/core package labels Feb 21, 2025
Copy link

vercel bot commented Feb 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ionic-framework ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 21, 2025 6:18pm

Copy link
Contributor

@thetaPC thetaPC left a 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];
Copy link
Contributor

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?

Comment on lines +2244 to +2262
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;
}
}
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package type: feature request a new feature, enhancement, or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants