From b0888fce768bbf427208884d19044944c48cef55 Mon Sep 17 00:00:00 2001 From: David Skewis Date: Thu, 20 Aug 2026 15:28:20 +0100 Subject: [PATCH 1/3] AG-18126 Restore the homepage code panel's own light-mode theme (#14921) The theme-builder panel was originally painted dark navy with a bespoke light-on-dark syntax palette, applied through Prism '.token.*' rules. When Prism was removed those rules went dead, and the earlier fix for this ticket resolved it by dropping the panel's identity altogether and following the site-wide code block colours in each mode. That lost the deliberate design: the panel is meant to read as an editor window sitting on the hero, not as another inline code block. Shiki's 'ag-docs' theme emits its token colours as 'var(--color-code-*)' references rather than literal hex, so the palette is settled in the DOM rather than at highlight time. Redefining those custom properties on the panel in light mode re-themes its highlighting alone, with no second Shiki theme to register and no hand-rolled markup. All thirteen properties the theme references are covered, so no token can fall back to the light palette and land dark-on-navy. Overriding '--color-fg-primary' also carries the window control dots, which mix from it. Dark mode already lands light-on-dark and is unchanged. Make the line number gutter colour overridable so the panel can recolour it without a specificity fight; the fallbacks are the previous values, so every other code block renders identically. --- .../ThemeBuilderExample.module.scss | 24 +++++++++++++++---- .../components/code/CodeHighlight.module.scss | 7 +++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/documentation/ag-grid-docs/src/components/theme-builder-homepage/ThemeBuilderExample.module.scss b/documentation/ag-grid-docs/src/components/theme-builder-homepage/ThemeBuilderExample.module.scss index 57d1fa620ae..7351d8b21a8 100644 --- a/documentation/ag-grid-docs/src/components/theme-builder-homepage/ThemeBuilderExample.module.scss +++ b/documentation/ag-grid-docs/src/components/theme-builder-homepage/ThemeBuilderExample.module.scss @@ -110,8 +110,6 @@ .codeBlockWrapper { position: relative; - // Follow the site-wide code block background so the panel matches every other code block - // in the current mode. Shiki colours its tokens for the mode, not for this panel. background-color: var(--color-code-background); border-radius: 8px; padding: 16px; @@ -136,6 +134,26 @@ background-color: color-mix(in srgb, var(--color-white), var(--color-bg-primary) 90%); } + // The panel keeps its own dark identity in light mode. Shiki's 'ag-docs' theme emits token + // colours as 'var(--color-code-*)', so redefining them here re-themes this panel alone. + html:not(#{$selector-darkmode}) & { + --color-code-background: #111a4a; + --color-fg-primary: rgb(255 255 255 / 90%); + --color-code-comment: rgb(255 255 255 / 42%); + --color-code-tag: rgb(255 255 255 / 90%); + --color-code-keyword: #f472b6; + --color-code-class-name: #f472b6; + --color-code-string: #7dd3fc; + --color-code-symbol: #7dd3fc; + --color-code-selector: #7dd3fc; + --color-code-function: var(--color-brand-200); + --color-code-property: #a2a6ba; + --color-code-punctuation: #81869f; + --color-code-deleted: #81869f; + --color-code-line-number: rgb(255 255 255 / 90%); + --color-code-line-number-opacity: 1; + } + pre { height: 100%; margin: 26px 0 16px 0; @@ -157,8 +175,6 @@ width: 12px; height: 12px; border-radius: 50%; - // Tints against the panel in either mode: dark dots on the light code background, and the - // previous white dots once the foreground colour flips in dark mode. background-color: color-mix(in srgb, var(--color-fg-primary) 30%, transparent); } diff --git a/external/ag-website-shared/src/components/code/CodeHighlight.module.scss b/external/ag-website-shared/src/components/code/CodeHighlight.module.scss index 8d391e18653..c405c4e9132 100644 --- a/external/ag-website-shared/src/components/code/CodeHighlight.module.scss +++ b/external/ag-website-shared/src/components/code/CodeHighlight.module.scss @@ -41,11 +41,12 @@ margin-left: -3.5em; margin-right: 1em; text-align: right; - color: var(--color-util-brand-500); - opacity: 0.33; + // Overridable by blocks that paint their own palette. + color: var(--color-code-line-number, var(--color-util-brand-500)); + opacity: var(--color-code-line-number-opacity, 0.33); #{$selector-darkmode} & { - color: var(--color-fg-primary); + color: var(--color-code-line-number, var(--color-fg-primary)); } } } From e3c47e9fa7e4673f51e5112bf8efc0db5bd69ce1 Mon Sep 17 00:00:00 2001 From: "ag-jira-agent-ci[bot]" <286720198+ag-jira-agent-ci[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:49:20 +0100 Subject: [PATCH 2/3] AG-17283 Render the filter search icon from the icon set (ag-icon-search) (#14918) * AG-17283 Render the filter search icon from the icon set Replace the hardcoded base64 magnifying-glass SVG in the filter search inputs with a real .ag-icon.ag-icon-search element, created the same way the toolbar Find input creates its own, so both render the same glyph and track future icon-set updates. - AgInputTextField.setSearchIcon() now creates (once, lazily) an aria-hidden .ag-input-field-search-icon wrapper holding the icon-set icon, and shows/hides it on toggle. - input-style-base.css drops the base64 mask-image ::before rule and positions the wrapper instead, keeping the public inputIconColor param alive by redefining --ag-icon-color (an inherited colour cannot reach .ag-icon, which sets colour on itself). - The legacy themes drop the quartz-only base64 blocks and gain one shared positioning rule in the base, so every legacy theme renders the icon consistently. Co-Authored-By: Claude Opus 5 (1M context) * AG-17283 Assert the icon-set search icon in the filter input tests Nothing in the repo asserted ag-icon-search before this change. Cover the floating filter, the text filter body and the set-filter mini filter, and correct the custom-icons docs comment now that the glyph is shared with the filter search inputs. Co-Authored-By: Claude Opus 5 (1M context) * AG-17283 Keep the search icon aria-hidden when toggled _setDisplayed sets aria-hidden from the displayed flag, which un-hid the decorative icon from screen readers when it was shown. Pass skipAriaHidden. Co-Authored-By: Claude Opus 5 (1M context) * AG-17283 Win the cascade for the legacy search input padding * AG-17283 Beat the Alpine and Balham input padding too --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) --- .../src/internal/base/parts/_widgets.scss | 22 ++++++++++++ .../src/internal/themes/quartz/_index.scss | 36 ------------------- .../src/content/docs/custom-icons/index.mdoc | 2 +- .../src/agWidgets/agInputTextField.ts | 25 +++++++++++++ .../parts/input-style/input-style-base.css | 35 +++++++++--------- .../src/filters/filter-clear-button.test.ts | 3 ++ .../src/filters/floating-filters.test.ts | 5 +++ 7 files changed, 72 insertions(+), 56 deletions(-) diff --git a/community-modules/styles/src/internal/base/parts/_widgets.scss b/community-modules/styles/src/internal/base/parts/_widgets.scss index ea7e924df90..e85c13e3397 100644 --- a/community-modules/styles/src/internal/base/parts/_widgets.scss +++ b/community-modules/styles/src/internal/base/parts/_widgets.scss @@ -106,6 +106,28 @@ ); } + // the glyph itself comes from the theme's icon font, as `.ag-icon-search` + .ag-input-field-search-icon { + position: absolute; + display: inline-flex; + align-items: center; + opacity: 0.5; + pointer-events: none; + inset-inline-start: var(--ag-grid-size); + } + + // the `.ag-ltr`/`.ag-rtl` prefix and the input's own class are what lift this above each theme's + // `ag.text-input` padding — `.ag-theme-x .ag-ltr input[class^='ag-'][type='text']` in Alpine and + // Balham — which is emitted after this file and would otherwise win the cascade + .ag-input-field-search input.ag-input-field-input[class^='ag-'][type='text'], + .ag-input-field-search input.ag-input-field-input[class^='ag-'][type='number'] { + @include ag.unthemed-rtl( + ( + padding-left: calc(var(--ag-icon-size) + var(--ag-grid-size) * 2), + ) + ); + } + input[class^='ag-'][type='number']:not(.ag-number-field-input-stepper) { -moz-appearance: textfield; &::-webkit-outer-spin-button, diff --git a/community-modules/styles/src/internal/themes/quartz/_index.scss b/community-modules/styles/src/internal/themes/quartz/_index.scss index 1bb00b2d72e..953bd32a063 100644 --- a/community-modules/styles/src/internal/themes/quartz/_index.scss +++ b/community-modules/styles/src/internal/themes/quartz/_index.scss @@ -628,35 +628,6 @@ opacity: 0.35; } - .ag-input-field-search { - .ag-input-wrapper::before { - position: absolute; - display: block; - @include ag.theme-rtl( - ( - margin-left: var(--ag-grid-size), - ) - ); - width: 12px; - height: 12px; - background-image: url('data:image/svg+xml;charset=utf-8;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIxMiIgaGVpZ2h0PSIxMiIgZmlsbD0ibm9uZSIgc3Ryb2tlPSIjMDAwIiBzdHJva2UtbGluZWNhcD0icm91bmQiIHN0cm9rZS1saW5lam9pbj0icm91bmQiIHN0cm9rZS13aWR0aD0iMS41Ij48cGF0aCBkPSJNNS4zIDlhMy43IDMuNyAwIDEgMCAwLTcuNSAzLjcgMy43IDAgMCAwIDAgNy41Wk0xMC41IDEwLjUgOC4zIDguMiIvPjwvc3ZnPg=='); - background-position: 50% 50%; - background-size: contain; - opacity: 40%; - content: ''; - filter: var(--ag-icon-filter); - } - - input.ag-text-field-input, - input.ag-number-field-input { - @include ag.theme-rtl( - ( - padding-left: 26px, - ) - ); - } - } - .ag-column-select-add-group-indent { @include ag.theme-rtl( ( @@ -695,13 +666,6 @@ } .ag-theme-quartz-dark { - .ag-input-field-search { - .ag-input-wrapper::before { - opacity: 66%; - filter: invert(100%); - } - } - .ag-chart-menu { background-color: color-mix(in srgb, rgba(24, 39, 50, 0.3), var(--ag-background-color) 30%); } diff --git a/documentation/ag-grid-docs/src/content/docs/custom-icons/index.mdoc b/documentation/ag-grid-docs/src/content/docs/custom-icons/index.mdoc index 874eaa8a80c..4c1064ce4d4 100644 --- a/documentation/ag-grid-docs/src/content/docs/custom-icons/index.mdoc +++ b/documentation/ag-grid-docs/src/content/docs/custom-icons/index.mdoc @@ -492,7 +492,7 @@ chartsThemeNext: 'next' // download chart chartsDownload: 'save' -// icon used in the find toolbar input +// icon used in the find toolbar input and in filter search inputs search: 'search' // file input overlay diff --git a/packages/ag-grid-community/src/agWidgets/agInputTextField.ts b/packages/ag-grid-community/src/agWidgets/agInputTextField.ts index f20d93220cf..c92d87ffe90 100644 --- a/packages/ag-grid-community/src/agWidgets/agInputTextField.ts +++ b/packages/ag-grid-community/src/agWidgets/agInputTextField.ts @@ -55,6 +55,7 @@ export class AgInputTextField< AgInputTextFieldEvent | TEventType > { private eClearButton: HTMLButtonElement | undefined; + private eSearchIcon: HTMLElement | undefined; private clearButtonEnabled: boolean = false; constructor(config?: TConfig, className = 'ag-text-field', inputType = 'text') { @@ -113,6 +114,14 @@ export class AgInputTextField< public setSearchIcon(searchIcon: boolean): this { this.toggleCss('ag-input-field-search', searchIcon); + if (searchIcon && !this.eSearchIcon) { + this.createSearchIcon(); + } + const { eSearchIcon } = this; + if (eSearchIcon) { + // decorative: it stays aria-hidden whether or not it is displayed + _setDisplayed(eSearchIcon, searchIcon, { skipAriaHidden: true }); + } return this; } @@ -173,6 +182,22 @@ export class AgInputTextField< this.eClearButton = eClearButton; } + private createSearchIcon(): void { + const eIcon = this.beans.iconSvc.createIconNoSpan('search'); + if (!eIcon) { + return; + } + const eSearchIcon = _createAgElement({ + tag: 'span', + cls: 'ag-input-field-search-icon', + attrs: { 'aria-hidden': 'true' }, + }); + eSearchIcon.appendChild(eIcon); + // the search icon leads the input; the clear button trails it + this.eWrapper.insertBefore(eSearchIcon, this.eWrapper.firstChild); + this.eSearchIcon = eSearchIcon; + } + private refreshClearButton(): void { const { eClearButton, eInput } = this; if (!eClearButton || !eInput) { diff --git a/packages/ag-grid-community/src/theming/parts/input-style/input-style-base.css b/packages/ag-grid-community/src/theming/parts/input-style/input-style-base.css index 72b36938962..53ac1d71c18 100644 --- a/packages/ag-grid-community/src/theming/parts/input-style/input-style-base.css +++ b/packages/ag-grid-community/src/theming/parts/input-style/input-style-base.css @@ -71,29 +71,26 @@ } } -/* icon for search inputs */ -:where(.ag-input-field-search) { - .ag-input-wrapper::before { - position: absolute; - display: block; - margin-left: var(--ag-spacing); - width: 12px; - height: 12px; - color: var(--ag-input-icon-color); - background-color: currentcolor; +/* icon for search inputs - the glyph itself comes from the active icon set, as `.ag-icon-search` */ +.ag-input-field-search-icon { + position: absolute; + inset-inline-start: var(--ag-spacing); + display: inline-flex; + align-items: center; + opacity: 0.5; + pointer-events: none; - /* masked background colour, which must be preserved when printing */ - print-color-adjust: exact; - mask-image: url('data:image/svg+xml;charset=utf-8;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIxMiIgaGVpZ2h0PSIxMiIgZmlsbD0ibm9uZSIgc3Ryb2tlPSIjMDAwIiBzdHJva2UtbGluZWNhcD0icm91bmQiIHN0cm9rZS1saW5lam9pbj0icm91bmQiIHN0cm9rZS13aWR0aD0iMS41Ij48cGF0aCBkPSJNNS4zIDlhMy43IDMuNyAwIDEgMCAwLTcuNSAzLjcgMy43IDAgMCAwIDAgNy41Wk0xMC41IDEwLjUgOC4zIDguMiIvPjwvc3ZnPg=='); - mask-position: center; - mask-repeat: no-repeat; - opacity: 0.5; - content: ''; - } + /* + * `inputIconColor` must be applied by redefining the icon's own custom property: `.ag-icon` sets + * `color: var(--ag-icon-color)` on itself, so an inherited `color` here would never reach the glyph. + */ + --ag-icon-color: var(--ag-input-icon-color); +} +:where(.ag-input-field-search) { .ag-text-field-input, .ag-number-field-input { - padding-left: calc(var(--ag-spacing) * 1.5 + 12px); + padding-inline-start: calc(var(--ag-icon-size) + var(--ag-spacing) * 2); } } diff --git a/testing/behavioural/src/filters/filter-clear-button.test.ts b/testing/behavioural/src/filters/filter-clear-button.test.ts index 8fcbb3ace31..44fad02024d 100644 --- a/testing/behavioural/src/filters/filter-clear-button.test.ts +++ b/testing/behavioural/src/filters/filter-clear-button.test.ts @@ -40,6 +40,8 @@ describe('Filter input clear button', () => { const input = harness.input('text'); expect(input.closest('.ag-input-field')!.classList.contains('ag-input-field-search')).toBe(true); + // the magnifying glass comes from the active icon set, not a bespoke inline SVG + expect(input.closest('.ag-input-field')!.querySelector('.ag-icon.ag-icon-search')).not.toBeNull(); const clearButton = input.parentElement!.querySelector('.ag-input-field-clear-button')!; expect(clearButton.classList.contains('ag-hidden')).toBe(false); fireEvent.mouseDown(clearButton); @@ -131,6 +133,7 @@ describe('Filter input clear button', () => { const miniFilterInput = document.querySelector('.ag-mini-filter input[type="text"]')!; expect(miniFilterInput.closest('.ag-input-field')!.classList.contains('ag-input-field-search')).toBe(true); + expect(miniFilterInput.closest('.ag-input-field')!.querySelector('.ag-icon.ag-icon-search')).not.toBeNull(); const clearButton = document.querySelector('.ag-mini-filter .ag-input-field-clear-button')!; expect(clearButton.classList.contains('ag-hidden')).toBe(false); fireEvent.mouseDown(clearButton); diff --git a/testing/behavioural/src/filters/floating-filters.test.ts b/testing/behavioural/src/filters/floating-filters.test.ts index 1d53e9a48f1..2bca68501d2 100644 --- a/testing/behavioural/src/filters/floating-filters.test.ts +++ b/testing/behavioural/src/filters/floating-filters.test.ts @@ -310,6 +310,11 @@ describe('Floating Filters', () => { expect(textFilter.closest('.ag-input-field')!.classList.contains('ag-input-field-search')).toBe(true); expect(textFilter.closest('.ag-floating-filter-search-icon')).toBeNull(); + // the magnifying glass comes from the active icon set, not a bespoke inline SVG + const searchIcon = textFilter.closest('.ag-input-field')!.querySelector('.ag-icon.ag-icon-search')!; + expect(searchIcon).not.toBeNull(); + expect(searchIcon.closest('.ag-input-field-search-icon')!.getAttribute('aria-hidden')).toBe('true'); + // Custom placeholder is presentational only — it applies no filter, so all rows remain. expect(api.getDisplayedRowCount()).toBe(3); }); From 88a5e5b2a65d290a69984e5d024ae44eefe769c5 Mon Sep 17 00:00:00 2001 From: Guilherme Lopes Date: Thu, 20 Aug 2026 13:17:57 -0300 Subject: [PATCH 3/3] [Focus] - Improved Focus Handling (#14919) * [Filters] - Focus trap should not focus tabindex -1 * improved logic to calculate index of managed items * fixed keyboard navigation editable/non-editable --- .../src/filter/filterMenuFactory.ts | 4 +- .../floatingFilter/headerFilterCellCtrl.ts | 2 +- .../src/navigation/navigationService.ts | 10 +- .../overlays/overlayWrapperComponent.ts | 3 +- .../src/rendering/row/fullWidthRowFeature.ts | 8 +- .../ag-grid-community/src/utils/gridFocus.ts | 2 +- .../advancedFilterHeaderComp.ts | 11 +- .../src/agStack/agTabbedLayout.ts | 2 +- .../advancedSettingsMenuFactory.ts | 2 +- .../src/menu/columnChooserFactory.ts | 2 +- .../src/sideBar/agSideBar.ts | 2 +- .../src/widgets/AgPillContainer.ts | 2 +- .../src/widgets/pillDropZonePanel.ts | 14 +- .../src/focus/agManagedFocusFeature.ts | 6 +- packages/ag-stack/src/focus/tabGuardCtrl.ts | 2 +- packages/ag-stack/src/utils/focus.ts | 27 +-- .../cell-editing-regression.test.ts | 36 ++++ ...cell-editing-validation-navigation.test.ts | 36 ++++ .../filters/filter-menu-focus-trap.test.ts | 160 ++++++++++++++++++ 19 files changed, 290 insertions(+), 41 deletions(-) create mode 100644 testing/behavioural/src/filters/filter-menu-focus-trap.test.ts diff --git a/packages/ag-grid-community/src/filter/filterMenuFactory.ts b/packages/ag-grid-community/src/filter/filterMenuFactory.ts index c4d6fed029a..cfb6b8245f0 100644 --- a/packages/ag-grid-community/src/filter/filterMenuFactory.ts +++ b/packages/ag-grid-community/src/filter/filterMenuFactory.ts @@ -201,14 +201,14 @@ export class FilterMenuFactory extends BeanStub implements NamedBean, IMenuFacto if ( e.key !== KeyCode.TAB || e.defaultPrevented || - _findNextFocusableElement(this.beans, menu, false, e.shiftKey) + _findNextFocusableElement({ beans: this.beans, rootNode: menu, onlyUnmanaged: true, backwards: e.shiftKey }) ) { return; } e.preventDefault(); - _focusInto(menu, e.shiftKey); + _focusInto(menu, e.shiftKey, true); } private dispatchVisibleChangedEvent(visible: boolean, containerType: ContainerType, column?: AgColumn): void { diff --git a/packages/ag-grid-community/src/headerRendering/cells/floatingFilter/headerFilterCellCtrl.ts b/packages/ag-grid-community/src/headerRendering/cells/floatingFilter/headerFilterCellCtrl.ts index 68b24f1ec27..2f02f3e170a 100644 --- a/packages/ag-grid-community/src/headerRendering/cells/floatingFilter/headerFilterCellCtrl.ts +++ b/packages/ag-grid-community/src/headerRendering/cells/floatingFilter/headerFilterCellCtrl.ts @@ -156,7 +156,7 @@ export class HeaderFilterCellCtrl extends AbstractHeaderCellCtrl { - _findNextFocusableElement(beans, columnSelectPanel.getGui())?.focus({ + _findNextFocusableElement({ beans, rootNode: columnSelectPanel.getGui() })?.focus({ preventScroll: true, }); this.dispatchVisibleChangedEvent(true, column); diff --git a/packages/ag-grid-enterprise/src/sideBar/agSideBar.ts b/packages/ag-grid-enterprise/src/sideBar/agSideBar.ts index 22c46d0d36d..03ddd50b493 100644 --- a/packages/ag-grid-enterprise/src/sideBar/agSideBar.ts +++ b/packages/ag-grid-enterprise/src/sideBar/agSideBar.ts @@ -126,7 +126,7 @@ class AgSideBar extends Component implements ISideBar, FocusableContainer { let nextEl: HTMLElement | null = null; if (openPanel.contains(activeElement)) { - nextEl = _findNextFocusableElement(beans, openPanel, undefined, true); + nextEl = _findNextFocusableElement({ beans, rootNode: openPanel, backwards: true }); } else if (isTargetUnderManagedComponent(openPanel, target)) { nextEl = findFocusableElementBeforeTabGuard(openPanel, target); } diff --git a/packages/ag-grid-enterprise/src/widgets/AgPillContainer.ts b/packages/ag-grid-enterprise/src/widgets/AgPillContainer.ts index 59e345158d9..e120fe74155 100644 --- a/packages/ag-grid-enterprise/src/widgets/AgPillContainer.ts +++ b/packages/ag-grid-enterprise/src/widgets/AgPillContainer.ts @@ -117,7 +117,7 @@ export class AgPillContainer extends Component { activePill.focus(); } - const nextFocusableEl = _findNextFocusableElement(beans, eGui, false, isPrevious); + const nextFocusableEl = _findNextFocusableElement({ beans, rootNode: eGui, backwards: isPrevious }); if (nextFocusableEl) { nextFocusableEl.focus(); diff --git a/packages/ag-grid-enterprise/src/widgets/pillDropZonePanel.ts b/packages/ag-grid-enterprise/src/widgets/pillDropZonePanel.ts index 89d5c55747d..40079ed20a4 100644 --- a/packages/ag-grid-enterprise/src/widgets/pillDropZonePanel.ts +++ b/packages/ag-grid-enterprise/src/widgets/pillDropZonePanel.ts @@ -180,13 +180,14 @@ export abstract class PillDropZonePanel, TItem private onKeyDown(e: KeyboardEvent) { const { key } = e; - const isVertical = !this.horizontal; + const { beans, horizontal, gos } = this; + const isVertical = !horizontal; let isNext = key === KeyCode.DOWN; let isPrevious = key === KeyCode.UP; if (!isVertical) { - const isRtl = this.gos.get('enableRtl'); + const isRtl = gos.get('enableRtl'); isNext = (!isRtl && key === KeyCode.RIGHT) || (isRtl && key === KeyCode.LEFT); isPrevious = (!isRtl && key === KeyCode.LEFT) || (isRtl && key === KeyCode.RIGHT); } @@ -201,7 +202,7 @@ export abstract class PillDropZonePanel, TItem this.moveFocusedItem(isPrevious); } else { const root = this.getFocusableElement(); - const el = _findNextFocusableElement(this.beans, root, false, isPrevious); + const el = _findNextFocusableElement({ beans, rootNode: root, backwards: isPrevious }); if (el) { el.focus(); @@ -490,13 +491,14 @@ export abstract class PillDropZonePanel, TItem const resizeEnabled = this.resizeEnabled; const focusedIndex = this.getFocusedItem(); - const { eGridDiv } = this.beans; + const beans = this.beans; + const { eGridDiv: rootNode } = beans; const isKeyboardMode = _isKeyboardMode(); let alternateElement: HTMLElement | null = null; if (isKeyboardMode) { alternateElement = - _findNextFocusableElement(this.beans, eGridDiv) ?? - _findNextFocusableElement(this.beans, eGridDiv, false, true); + _findNextFocusableElement({ beans, rootNode }) ?? + _findNextFocusableElement({ beans, rootNode, backwards: true }); } this.toggleResizable(false); diff --git a/packages/ag-stack/src/focus/agManagedFocusFeature.ts b/packages/ag-stack/src/focus/agManagedFocusFeature.ts index 394743dc8f3..add3977a2b8 100644 --- a/packages/ag-stack/src/focus/agManagedFocusFeature.ts +++ b/packages/ag-stack/src/focus/agManagedFocusFeature.ts @@ -45,7 +45,11 @@ export class AgManagedFocusFeature< return; } - const nextRoot = _findNextFocusableElement(this.beans, this.eFocusable, false, e.shiftKey); + const nextRoot = _findNextFocusableElement({ + beans: this.beans, + rootNode: this.eFocusable, + backwards: e.shiftKey, + }); if (!nextRoot) { return; diff --git a/packages/ag-stack/src/focus/tabGuardCtrl.ts b/packages/ag-stack/src/focus/tabGuardCtrl.ts index 8a73c1f9ea1..39c04b418c0 100644 --- a/packages/ag-stack/src/focus/tabGuardCtrl.ts +++ b/packages/ag-stack/src/focus/tabGuardCtrl.ts @@ -341,7 +341,7 @@ export class AgTabGuardCtrl< } public getNextFocusableElement(backwards?: boolean): HTMLElement | null { - return _findNextFocusableElement(this.beans, this.eFocusableElement, false, backwards); + return _findNextFocusableElement({ beans: this.beans, rootNode: this.eFocusableElement, backwards }); } public forceFocusOutOfContainer(up: boolean = false): void { diff --git a/packages/ag-stack/src/utils/focus.ts b/packages/ag-stack/src/utils/focus.ts index 7113c439b51..f510eaf2c3c 100644 --- a/packages/ag-stack/src/utils/focus.ts +++ b/packages/ag-stack/src/utils/focus.ts @@ -140,23 +140,24 @@ export function _focusIntoTabbableFirst(rootNode: HTMLElement, up = false, exclu } /** @internal AG_GRID_INTERNAL - Not for public use. Can change / be removed at any time. */ -export function _findNextFocusableElement( - beans: UtilBeanCollection, - rootNode: HTMLElement, - onlyManaged?: boolean | null, - backwards?: boolean -): HTMLElement | null { - const focusable = _findFocusableElements(rootNode, onlyManaged ? ':not([tabindex="-1"])' : null); +export function _findNextFocusableElement(params: { + beans: UtilBeanCollection; + rootNode: HTMLElement; + onlyManaged?: boolean | null; + onlyUnmanaged?: boolean; + backwards?: boolean; +}): HTMLElement | null { + const { beans, rootNode, onlyManaged, onlyUnmanaged, backwards } = params; const activeEl = _getActiveDomElement(beans) as HTMLElement; - let currentIndex: number; - if (onlyManaged) { - currentIndex = focusable.findIndex((el) => el.contains(activeEl)); - } else { - currentIndex = focusable.indexOf(activeEl); + let focusable = _findFocusableElements(rootNode, onlyManaged ? ':not([tabindex="-1"])' : null); + if (onlyUnmanaged) { + focusable = focusable.filter((el) => el === activeEl || _getTabIndex(el) !== '-1'); } - const nextIndex = currentIndex + (backwards ? -1 : 1); + const activeIndex = onlyManaged ? focusable.findIndex((el) => el.contains(activeEl)) : focusable.indexOf(activeEl); + + const nextIndex = activeIndex + (backwards ? -1 : 1); if (nextIndex < 0 || nextIndex >= focusable.length) { return null; diff --git a/testing/behavioural/src/cell-editing/cell-editing-regression.test.ts b/testing/behavioural/src/cell-editing/cell-editing-regression.test.ts index 130c408ab3b..a36684b15bc 100644 --- a/testing/behavioural/src/cell-editing/cell-editing-regression.test.ts +++ b/testing/behavioural/src/cell-editing/cell-editing-regression.test.ts @@ -215,6 +215,42 @@ describe('Cell Editing Regression', () => { `); }); + test('tabbing out of the only editable cell moves focus to the adjacent cell', async () => { + const api = await gridMgr.createGridAndWait('myGrid', { + columnDefs: [ + { field: 'a' }, + { field: 'b', editable: (params) => params.node.rowIndex === 1 }, + { field: 'c' }, + ], + rowData: [ + { a: 'a0', b: 'b0', c: 'c0' }, + { a: 'a1', b: 'b1', c: 'c1' }, + ], + // without a focusable header, the backwards search has no target left once it + // exhausts the cells, which is the state where focus used to be dropped entirely + suppressHeaderFocus: true, + }); + const gridDiv = getGridElement(api)! as HTMLElement; + + const editableCell = await waitFor(() => getByTestId(gridDiv, agTestIdFor.cell('1', 'b'))); + + await userEvent.dblClick(editableCell); + await waitForInput(gridDiv, editableCell); + await userEvent.keyboard('{Shift>}{Tab}{/Shift}'); + + expect(api.getCellEditorInstances()).toHaveLength(0); + expect(getByTestId(gridDiv, agTestIdFor.cell('1', 'a'))).toHaveFocus(); + expect(api.getFocusedCell()?.column.getColId()).toBe('a'); + + await userEvent.dblClick(editableCell); + await waitForInput(gridDiv, editableCell); + await userEvent.keyboard('{Tab}'); + + expect(api.getCellEditorInstances()).toHaveLength(0); + expect(getByTestId(gridDiv, agTestIdFor.cell('1', 'c'))).toHaveFocus(); + expect(api.getFocusedCell()?.column.getColId()).toBe('c'); + }); + test('full-row editing closes empty editors when tabbing to next row', async () => { const api = await gridMgr.createGridAndWait('myGrid', { columnDefs: [{ field: 'make' }, { field: 'model' }, { field: 'model3' }], diff --git a/testing/behavioural/src/cell-editing/cell-editing-validation-navigation.test.ts b/testing/behavioural/src/cell-editing/cell-editing-validation-navigation.test.ts index 24d6d9d4c4f..d3d32729e50 100644 --- a/testing/behavioural/src/cell-editing/cell-editing-validation-navigation.test.ts +++ b/testing/behavioural/src/cell-editing/cell-editing-validation-navigation.test.ts @@ -338,6 +338,42 @@ describe('Cell editing validation interacting with navigation', () => { `); }); + test('singleCell block: Tab from an invalid cell does not navigate when no other cell is editable', async () => { + const rowData: PersonRow[] = [{ athlete: 'Alice', age: 23 }]; + const api = await gridsManager.createGridAndWait('nav-singleCell-block-only-editable', { + columnDefs: [ + { field: 'athlete', editable: false }, + { + field: 'age', + cellEditor: 'agNumberCellEditor', + cellEditorParams: { min: 0, max: 100 }, + editable: true, + }, + ], + rowData, + editType: 'singleCell', + invalidEditValueMode: 'block', + } satisfies GridOptions); + const gridElement = getGridElement(api)! as HTMLElement; + const user = userEvent.setup(); + + const ageCell = cell(api, 0, 'age'); + await user.dblClick(ageCell); + const ageInput = await waitForInput(gridElement, ageCell); + await user.clear(ageInput); + await user.type(ageInput, '999'); + + await user.keyboard('{Tab}'); + + // the block must hold even though no other editable cell exists: the editor stays + // open with no commit, and focus must not fall back to plain cell navigation + expect(editorCount(api)).toBeGreaterThan(0); + expect(cell(api, 0, 'age').querySelector('input')).toBeTruthy(); + expect(cell(api, 0, 'athlete')).not.toBe(document.activeElement); + expect(cell(api, 0, 'athlete').querySelector('input')).toBeFalsy(); + expect(rowData[0].age).toBe(23); + }); + // --- Arrow keys don't navigate (or commit) while an editor is open --- test('singleCell block: arrow keys while editing an invalid cell do not navigate or commit', async () => { diff --git a/testing/behavioural/src/filters/filter-menu-focus-trap.test.ts b/testing/behavioural/src/filters/filter-menu-focus-trap.test.ts new file mode 100644 index 00000000000..42c4216b1da --- /dev/null +++ b/testing/behavioural/src/filters/filter-menu-focus-trap.test.ts @@ -0,0 +1,160 @@ +import { waitFor } from '@testing-library/dom'; +import '@testing-library/jest-dom/vitest'; +import { userEvent } from '@testing-library/user-event'; +import { TestGridsManager } from 'ag-test-utils'; + +import type { IFilterComp } from 'ag-grid-community'; +import { ClientSideRowModelModule, CustomFilterModule } from 'ag-grid-community'; + +function createInput(className: string): HTMLInputElement { + const input = document.createElement('input'); + input.className = className; + return input; +} + +/** A focusable but not tabbable element, reachable only programmatically (not part of the Tab flow). */ +function createStaticPanel(): HTMLDivElement { + const panel = document.createElement('div'); + panel.className = 'test-static-panel'; + panel.tabIndex = -1; + panel.textContent = 'static panel'; + return panel; +} + +abstract class StaticPanelFilter implements IFilterComp { + private eGui!: HTMLElement; + + protected abstract createChildren(): HTMLElement[]; + + public init(): void { + this.eGui = document.createElement('div'); + this.eGui.append(...this.createChildren()); + } + + public getGui(): HTMLElement { + return this.eGui; + } + + public doesFilterPass(): boolean { + return true; + } + + public isFilterActive(): boolean { + return false; + } + + public getModel(): unknown { + return null; + } + + public setModel(): void {} +} + +class PanelAfterInputFilter extends StaticPanelFilter { + protected override createChildren(): HTMLElement[] { + return [createInput('test-filter-input'), createStaticPanel()]; + } +} + +class PanelBetweenInputsFilter extends StaticPanelFilter { + protected override createChildren(): HTMLElement[] { + return [createInput('test-first-input'), createStaticPanel(), createInput('test-second-input')]; + } +} + +class PanelBeforeInputFilter extends StaticPanelFilter { + protected override createChildren(): HTMLElement[] { + return [createStaticPanel(), createInput('test-filter-input')]; + } +} + +describe('Filter menu focus trap', () => { + const gridsManager = new TestGridsManager({ + modules: [ClientSideRowModelModule, CustomFilterModule], + }); + + afterEach(() => { + gridsManager.reset(); + }); + + async function openFilterMenu(colId: 'a' | 'b' | 'c'): Promise { + const api = await gridsManager.createGridAndWait('filterFocusTrapGrid', { + columnDefs: [ + { field: 'a', filter: PanelAfterInputFilter }, + { field: 'b', filter: PanelBetweenInputsFilter }, + { field: 'c', filter: PanelBeforeInputFilter }, + ], + rowData: [{ a: 1, b: 2, c: 3 }], + }); + + api.showColumnFilter(colId); + + return await waitFor(() => { + const menu = document.querySelector('.ag-menu'); + expect(menu).not.toBeNull(); + expect(menu!.querySelector('input')).not.toBeNull(); + return menu!; + }); + } + + test('Tab and Shift+Tab stay trapped when the only tabbable element is followed by a tabindex="-1" element', async () => { + const menu = await openFilterMenu('a'); + const user = userEvent.setup(); + const input = menu.querySelector('.test-filter-input')!; + + input.focus(); + + // nothing tabbable after the input: the trap must wrap instead of letting focus escape the menu + await user.tab(); + expect(input).toHaveFocus(); + + // the panel is not part of the Tab flow: wrapping backwards must not land on it + await user.tab({ shift: true }); + expect(input).toHaveFocus(); + }); + + test('Tab from a focused tabindex="-1" element wraps back into the menu', async () => { + const menu = await openFilterMenu('a'); + const user = userEvent.setup(); + const input = menu.querySelector('.test-filter-input')!; + const panel = menu.querySelector('.test-static-panel')!; + + panel.focus(); + expect(panel).toHaveFocus(); + + // nothing tabbable after the panel: native Tab would escape the menu, so the trap must wrap + await user.tab(); + expect(input).toHaveFocus(); + }); + + test('the tab flow moves around a tabindex="-1" element between tabbable elements', async () => { + const menu = await openFilterMenu('b'); + const user = userEvent.setup(); + const firstInput = menu.querySelector('.test-first-input')!; + const secondInput = menu.querySelector('.test-second-input')!; + const panel = menu.querySelector('.test-static-panel')!; + + // Shift+Tab from the panel goes to the previous tabbable element, not to a wrap target + panel.focus(); + await user.tab({ shift: true }); + expect(firstInput).toHaveFocus(); + + // Tab from the last tabbable element wraps to the first, skipping the panel + secondInput.focus(); + await user.tab(); + expect(firstInput).toHaveFocus(); + }); + + test('Shift+Tab from a tabindex="-1" element before all tabbable elements wraps to the last tabbable', async () => { + const menu = await openFilterMenu('c'); + const user = userEvent.setup(); + const input = menu.querySelector('.test-filter-input')!; + const panel = menu.querySelector('.test-static-panel')!; + + panel.focus(); + + // native Shift+Tab would escape the menu: the trap must intercept and wrap to the last tabbable + await user.tab({ shift: true }); + expect(input).toHaveFocus(); + }); +});