-
Notifications
You must be signed in to change notification settings - Fork 3.2k
docs(datetime): add multiple selection playground #2442
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
const datetime = useRef<null | HTMLIonDatetimeElement>(null); | ||
|
||
useEffect(() => { | ||
if(!datetime.current) return; | ||
datetime.current.value = ['2022-06-03', '2022-06-13', '2022-06-29']; | ||
}, []); |
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.
This is done to get around this issue, which also applies here: ionic-team/ionic-framework#25041
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.
Should we update that issue to mention it will affect ion-datetime
with multiple
? Also, should we add a comment in the playground so in the future when that issue is resolved, we can update this example and remove this code?
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.
Good call 👀 I updated the issue and will add a tech debt ticket.
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.
Ticket made: https://ionic-cloud.atlassian.net/browse/FW-1872 I skipped adding the comment since I didn't want it to show up in the actual docs site (and putting it in demo.html
felt weird when it's not relevant to that), but I think the Jira ticket should be good enough for tracking this.
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.
Optional: Thoughts on moving the multiple date selection up higher? Given the popularity of the feature ticket on GitHub, it seems like multiple date selection may be fairly common.
Otherwise this change looks great.
Works for me 👀 |
I moved it right underneath the Edit: Also removed the hard-coded dev build from the demo. |
PR for feature: ionic-team/ionic-framework#25514
Dev build:
6.1.14-dev.11657720710.1df41c89
The demo example is hard-coded to use the above dev build, but you'll need to install it on the Stackblitz examples manually.