Skip to content

fix(TimePicker): close time options when disabled - #12719

Open
Xapollon430 wants to merge 1 commit into
patternfly:mainfrom
Xapollon430:codex/time-picker-close-on-disable
Open

Xapollon430 wants to merge 1 commit into
patternfly:mainfrom
Xapollon430:codex/time-picker-close-on-disable

Conversation

@Xapollon430

@Xapollon430 Xapollon430 commented Nov 12, 2026 •

Copy link
Copy Markdown

What: Closes #12717

Close the time options when the picker becomes disabled and ignore option selections while it is disabled.

To reproduce, open TimePicker and disable it from the parent while the popup is still open. Try selecting 11:00. The selected time should stay unchanged and the change callback should not fire. Re-enable the picker and select the option again to check normal behavior.

Chrome recording

The regression fails on current main and passes with the fix. The DatePicker and TimePicker Jest suites passed together with 32 tests and one existing skip, using a local Jest configuration for the existing Babel transform, setup, and published workspace dependencies.

Summary by CodeRabbit

  • Bug Fixes
    • Disabled time pickers now close their options menu and clear the focused option, including when disabled while open.
    • Options cannot be selected while the picker is disabled. After re-enabling, the menu stays closed until reopened.

@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The TimePicker now initializes its options menu as closed when disabled. If it becomes disabled while open, it closes the menu and clears the focused index. Toggle and selection actions do not proceed while disabled. A test covers disabling, re-enabling, and selecting an option.

Changes

TimePicker disabled-state handling

Layer / File(s) Summary
Disabled menu behavior
packages/react-core/src/components/TimePicker/TimePicker.tsx, packages/react-core/src/components/TimePicker/__tests__/TimePicker.test.tsx
The TimePicker initializes and updates its menu state based on isDisabled. Its toggle and selection handlers return while disabled. The test verifies that disabling an open picker prevents selection, and that the option appears again only after reopening the picker.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: thatblindgeye, wise-king-sullyman

Merge Risk: Minimal · up to 88e92

This change makes an open TimePicker close when it is disabled, and it ignores option selections while disabled. No merge-blocking risk is identified, and a test covers the disable and re-enable flow.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title uses the conventional fix(TimePicker) format and names the main change: the TimePicker options close when the picker becomes disabled. The raw summary confirms componentDidUpdate closes the …
Linked Issues check Passed Issue #12717 requires two behaviors: (1) disabling an open TimePicker closes its options list, and (2) selecting an option while disabled must not change the time or fire onChange. The diff to package…
Out of Scope Changes check Passed All changed code is limited to the TimePicker component and its test file. Every change maps to issue #12717: the constructor initialization, the componentDidUpdate close logic, the onToggle and onSel…
Full details: Title check

Explanation

The title uses the conventional fix(TimePicker) format and names the main change: the TimePicker options close when the picker becomes disabled. The raw summary confirms componentDidUpdate closes the menu when disabled, and the new test checks that a previously visible option is hidden and cannot be selected. The title does not cover the onToggle/onSelect early returns, but that is a secondary detail.

Full details: Linked Issues check

Explanation

Issue #12717 requires two behaviors: (1) disabling an open TimePicker closes its options list, and (2) selecting an option while disabled must not change the time or fire onChange. The diff to packages/react-core/src/components/TimePicker/TimePicker.tsx covers both. (1) componentDidUpdate now checks isDisabled && isTimeOptionsOpen, then sets isTimeOptionsOpen to false, clears focusedIndex, and calls this.props.setIsOpen(false). The constructor also initializes isTimeOptionsOpen as isOpen && !isDisabled. (2) onSelect returns early when this.props.isDisabled is true, so the time state and onChange are not reached. The test file adds a case that clicks the previously visible 11:00 option after disabling, expects no onChange call, and then verifies normal selection after re-enabling. The test was not executed during this assessment. The static code evidence supports the requirements, so no unmet coding requirement is found.

Full details: Out of Scope Changes check

Explanation

All changed code is limited to the TimePicker component and its test file. Every change maps to issue #12717: the constructor initialization, the componentDidUpdate close logic, the onToggle and onSelect disabled guards, and the regression test. No unrelated refactoring or feature change is visible in the reviewed diff. The raw summary reports no changes to exported declarations.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: acfe8733-6bb4-4ae3-bc6e-4f91f575691b
Commits

Reviewing files that changed from the base of the PR and between 75b6b80 and 88e9203.

Files selected for processing: 2

  • packages/react-core/src/components/TimePicker/TimePicker.tsx
  • packages/react-core/src/components/TimePicker/__tests__/TimePicker.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

This branch has not been deployed

No deployments
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.

TimePicker options can change the time after the picker becomes disabled

1 participant