Skip to content

Conversation

@Tucchhaa
Copy link
Contributor

No description provided.

@Tucchhaa Tucchhaa self-assigned this Jan 16, 2026
@Tucchhaa Tucchhaa added the 26_1 label Jan 16, 2026
private createRecurrenceRuleGroup(): GroupItem {
// Change of frequency editor's value causes rerender of the recurrencePatternGroup.
// To prevent focus loss in this editor, we use this flag.
let needRestoreFrequencyEditorFocus = false;
Copy link
Contributor Author

@Tucchhaa Tucchhaa Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason of focus loss is because recurrencePatternGroup group gets rerendered when updateDayEditorsVisibility() is called.

Another solution is to wrap weekGroup, monthGroup and yearGroup into different group here.

I have decided to use this flag, because it's used in very narrow scope and because form items already have very deep nesting (docs).

But if you have any objections, let's discuss it :)

@Tucchhaa Tucchhaa requested a review from sjbur January 20, 2026 04:56
@Tucchhaa Tucchhaa marked this pull request as ready for review January 20, 2026 08:23
@Tucchhaa Tucchhaa requested a review from a team as a code owner January 20, 2026 08:23
Copilot AI review requested due to automatic review settings January 20, 2026 08:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes keyboard navigation (KBN) issues in the Scheduler's appointment form, specifically focusing on proper focus management when switching between the main form and recurrence settings form.

Changes:

  • Replaced tabindex="-1" with the inert HTML attribute for better accessibility when hiding form groups
  • Added automatic focus management when switching between form groups
  • Fixed focus loss issue in the frequency editor when its value changes
  • Added visual focus state styling for the recurrence settings button in Fluent theme

Reviewed changes

Copilot reviewed 7 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/testcafe-models/scheduler/appointment/popup.ts Added selectors for back button, recurrence start date input, and frequency editor to support new functional tests
packages/devextreme/js/__internal/scheduler/appointment_popup/m_recurrence_form.ts Added CSS class to recurrence start date editor and implemented focus restoration logic for frequency editor to prevent focus loss during value changes
packages/devextreme/js/__internal/scheduler/appointment_popup/m_form.ts Replaced tabindex attribute with inert attribute for form group visibility, added focusFirstFocusableInGroup method, and reordered toolbar updates to occur before focus management
packages/devextreme/js/__internal/scheduler/appointment_popup/appointment_popup.test.ts Updated existing tests to check for inert attribute instead of tabindex, and added new tests to verify frequency editor focus behavior
packages/devextreme-scss/scss/widgets/fluent/scheduler/_index.scss Added focus state styling with outline for recurrence settings button in Fluent theme
e2e/testcafe-devextreme/tests/scheduler/common/appointmentForm/form.visual.ts Added visual regression test for recurrence settings button focus state
e2e/testcafe-devextreme/tests/scheduler/common/appointmentForm/form.functional.ts Added functional tests to verify focus behavior when navigating between main and recurrence forms

frequencyEditorInputElement.click();
jest.useFakeTimers();
keydown(frequencyEditorInputElement, 'ArrowDown');
jest.runAllTimers();
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test uses jest.useFakeTimers() but doesn't restore real timers before completing. While the afterEach hook does restore real timers globally, it's better practice to restore timers explicitly at the end of the test to avoid potential test isolation issues. Consider adding jest.useRealTimers() after line 1720 or wrapping the fake timer usage in a try-finally block.

Suggested change
jest.runAllTimers();
jest.runAllTimers();
jest.useRealTimers();

Copilot uses AI. Check for mistakes.
@Tucchhaa Tucchhaa merged commit 80d9e32 into DevExpress:26_1 Jan 20, 2026
99 of 100 checks passed
@Tucchhaa Tucchhaa deleted the form_kbn_26_1 branch January 20, 2026 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants