fix: calendar picker open/close flicker and chevron buttons#827
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR routes Popover event reasons through DatePicker into usePickerPopover and adds logic to ignore Sequence Diagram(s)sequenceDiagram
participant Input as DatePicker Trigger
participant Popover as Popover
participant Hook as usePickerPopover
Input->>Popover: click -> Popover.onOpenChange(open=true,eventDetails{reason:'trigger-press'})
Popover->>Hook: onOpenChange(open=true, reason='trigger-press')
alt open === true
Hook-->>Popover: accept open (no suppression)
else open === false and reason === 'trigger-press'
Hook-->>Popover: ignore close (suppress)
end
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/raystack/components/calendar/use-picker-popover.ts (1)
141-167:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTear down engagement on real close events.
onOpenChange(false, ...)now closes the popover without callingdisengage(). If the picker was already engaged,isEngagedRefstaystrueand the documentmouseuplistener stays armed, so the next input focus is ignored byhandleInputFocus()and keyboard reopen-on-focus breaks after an Escape/outside close.Suggested fix
- const onOpenChange = useCallback((open?: boolean, reason?: string) => { + const onOpenChange = useCallback((open?: boolean, reason?: string) => { // Year/month dropdown opening inside the popover triggers an open-change // we don't want; swallow it and consume the flag. if (isDropdownOpenRef.current) { isDropdownOpenRef.current = false; return; } @@ if (reason === 'trigger-press' && open === false) return; @@ if (open === true && isEngagedRef.current && isOpenRef.current) return; + if (open === false) { + disengage(); + return; + } setIsOpen(Boolean(open)); - }, []); + }, [disengage]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/calendar/use-picker-popover.ts` around lines 141 - 167, onOpenChange currently closes the popover (setIsOpen(false)) without calling disengage(), leaving isEngagedRef true and the document mouseup listener active; update onOpenChange to call disengage() whenever the popover is actually being closed (i.e., when open is false and the close isn't an ignored trigger-press or dropdown swallow) so that isEngagedRef is reset and the mouseup listener is removed; locate the onOpenChange callback and invoke the disengage() helper immediately before or instead of setIsOpen(false) for real close events (but keep the existing early returns for isDropdownOpenRef and the trigger-press ignore).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/raystack/components/calendar/calendar.tsx`:
- Line 129: The disabled prop currently uses nullish coalescing
(disabled={loadingData ?? props.disabled}) which always returns loadingData
(false) and ignores props.disabled; change both PreviousMonthButton and
NextMonthButton to combine conditions so the button is disabled when either
loadingData is true or props.disabled is true (i.e., use a logical OR between
loadingData and props.disabled) to ensure DayPicker’s disabled state is
respected.
- Line 130: The disabled visual state only checks loadingData but the actual
disabled prop is computed as loadingData || props.disabled, so update the
className logic in both PreviousMonthButton and NextMonthButton to use the same
combined disabled value (e.g., the existing disabled variable or compute
disabled = loadingData || props.disabled) instead of only loadingData so the
styles.disabled class is applied whenever the button is disabled by either
condition.
In `@packages/raystack/components/calendar/use-picker-popover.ts`:
- Around line 148-159: The unconditional early return for reason ===
'trigger-press' in usePickerPopover changes behavior for all consumers; make
this behavior opt-in by adding a hook option (e.g., ignoreTriggerPress: boolean,
default false) to usePickerPopover and guard the existing if (reason ===
'trigger-press' && open === false) return; behind that option, then update
DatePicker to call usePickerPopover({ ..., ignoreTriggerPress: true }) so only
the DatePicker opts into ignoring trigger-press closes.
---
Outside diff comments:
In `@packages/raystack/components/calendar/use-picker-popover.ts`:
- Around line 141-167: onOpenChange currently closes the popover
(setIsOpen(false)) without calling disengage(), leaving isEngagedRef true and
the document mouseup listener active; update onOpenChange to call disengage()
whenever the popover is actually being closed (i.e., when open is false and the
close isn't an ignored trigger-press or dropdown swallow) so that isEngagedRef
is reset and the mouseup listener is removed; locate the onOpenChange callback
and invoke the disengage() helper immediately before or instead of
setIsOpen(false) for real close events (but keep the existing early returns for
isDropdownOpenRef and the trigger-press ignore).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3604b79b-ac91-4bb5-a013-0327e3752b56
📒 Files selected for processing (4)
packages/raystack/components/calendar/__tests__/date-picker.test.tsxpackages/raystack/components/calendar/calendar.tsxpackages/raystack/components/calendar/date-picker.tsxpackages/raystack/components/calendar/use-picker-popover.ts
Shreyag02
requested changes
Jun 3, 2026
Shreyag02
approved these changes
Jun 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
trigger-pressclose events inusePickerPopoverso the date picker no longer flickers shut on the first input click (focus opened it, then the same click's trigger toggle closed it).reasonthroughDatePicker→usePickerPopover.onOpenChangeso close ownership stays with outside-click/blur/Enter/day-select logic while Escape and outside-press still flow through.Chevroncomponent to dedicatedPreviousMonthButton/NextMonthButton, preserving disabled-state styling and respecting RDP's own disabled flag.captionLayout/requiredcomments fromDatePickerafter the calendar prop wiring change.