Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hmnd
Copy link

@hmnd hmnd commented Apr 22, 2025

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.

Copy link
Contributor

github-actions bot commented Apr 28, 2025

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
bits-ui ✅ Ready (View Log) Visit Preview 2cedeba

@huntabyte
Copy link
Owner

huntabyte commented Apr 28, 2025

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.

@hmnd
Copy link
Author

hmnd commented Apr 28, 2025

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.

@huntabyte
Copy link
Owner

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!

@hmnd
Copy link
Author

hmnd commented Apr 28, 2025

Happy to discuss/provide feedback in chat on discord if preferred too - @.hmnd on there :)

Thanks!

@hmnd hmnd force-pushed the feature/pr-1379 branch from ca3655d to 2cedeba Compare May 16, 2025 02:54
Copy link

changeset-bot bot commented May 16, 2025

🦋 Changeset detected

Latest commit: 2cedeba

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
bits-ui Minor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants