Skip to content

feat(ion-datetime): add hidden-days when theme:ionic #30205

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

Closed
wants to merge 1 commit into from

Conversation

JoaoFerreira-FrontEnd
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeap, I'll fix it

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getDaysOfMonth doesn't have value of month or year

@thetaPC
Copy link
Contributor

thetaPC commented May 23, 2025

@JoaoFerreira-FrontEnd what's the status of this PR? Can we close it?

Copy link
Contributor Author

@JoaoFerreira-FrontEnd JoaoFerreira-FrontEnd left a comment

Choose a reason for hiding this comment

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

@thetaPC this was done under the context of another PR: #30262

That said I'll close this as suggested. Thanks

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 Author

Choose a reason for hiding this comment

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

yeap, I'll fix it

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 Author

Choose a reason for hiding this comment

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

getDaysOfMonth doesn't have value of month or year

@thetaPC thetaPC deleted the ROU-11118 branch May 27, 2025 15:10
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