-
-
Notifications
You must be signed in to change notification settings - Fork 156
feat(Calendar): add onVisibleMonthsChange callback to calendars and date pickers #1382
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
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
Hey there @hmnd!
We typically don't expose listeners/handlers for properties that we don't adjust internally, meaning the visibleMonths
prop is read-only from Bits UI's perspective. The developer passes that prop and can control it in userland, meaning they can hook into their specific implementation.
To determine what months are visible, you can use a combination of the placeholder
and visibleMonths
to determine that.
Hey @huntabyte, thanks for taking a look at this.
As far as I'm aware, visibleMonths
isn't currently exposed or controllable from userland. Because of that, I followed the implementation of onStartValueChange
instead, internally exposing months to the svelte components to achieve that. Placeholder is also not terribly helpful, as it doesn't provide the end date in range calendars and doesn't provide the actual visible range (although this can technically by calculated by the developer, but that feels like unnecessary doubling of effort).
Thus I think either a handler like I've implemented or a read-only binding to months
or visibleMonths
is necessary.
Ah, I see what you mean. I was thinking of numberOfMonths
instead of visualMonths
. Let me review this and think on it a little bit!
Happy to discuss/provide feedback in chat on discord if preferred too - @.hmnd
on there :)
Thanks!
ca3655d
to
2cedeba
Compare
🦋 Changeset detectedLatest commit: 1a2c890 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
2cedeba
to
4d63616
Compare
4d63616
to
1a2c890
Compare
Uh oh!
There was an error while loading. Please reload this page.
Resolves part of #1379
Adds
onVisibleMonthsChange
callback to both calendars and both date pickers. This allows end users to react to the displayed dates changing, eg. in order to load relevant availability data as the user navigates.I followed the approach currently used for the
onStartValueChange
callback, but am open to feedback if a different approach might be better.I ran into an issue where setting the initial months within the constructor on the boxed months value wouldn't trigger an update to relevant getters server-side. That resulted in a blank calendar rendering server side before populating client side. My workaround is to have an internal getter for months that defaults to the initial months when the boxed months array is blank.