fix: tab navigation not working in Fixed Layout due to event listener timing issue (#41256)

## Problem

Tab navigation between input widgets was not working in Fixed Layout
applications. Users reported that pressing the Tab key would not move
focus to the next input widget in the expected order (top-to-bottom,
left-to-right), instead following the browser's default DOM-based tab
order.

This issues was raised by an Enterprise user
[here](https://theappsmith.slack.com/archives/C0341RERY4R/p1758112042665109)

## Root Cause

The issue was caused by a **timing problem** in the `useWidgetFocus`
hook:

1. The `useEffect` hook was running immediately when the component
mounted
2. However, the canvas element ref (`ref.current`) was set later via the
React ref callback
3. This caused the event listeners for Tab navigation to never be
attached, as `ref.current` was `null` when `useEffect` ran
4. Without the custom Tab event listeners, the browser fell back to its
default tab navigation behavior

## Solution

Refactored the `useWidgetFocus` hook to attach event listeners
**immediately when the ref is set**, rather than waiting for a
`useEffect` that runs too early:

### Before (Broken):
```typescript
useEffect(() => {
  if (!ref.current) return; //  Always true - ref not set yet
  
  const handleKeyDown = (event: KeyboardEvent) => {
    if (event.key === "Tab") handleTab(event);
  };
  
  ref.current.addEventListener("keydown", handleKeyDown);
}, []); //  Runs before ref is set
```

### After (Fixed):
```typescript
const setRef = useCallback((node: HTMLElement | null) => {
  if (node === null) return;
  if (ref.current === node) return;
  
  ref.current = node;
  attachEventListeners(node); //  Attach immediately when ref is set
}, [attachEventListeners]);
```

## Why This Solution Works

1. **Correct Timing**: Event listeners are now attached immediately when
React calls the ref callback with the DOM element
2. **No Race Conditions**: Eliminates the timing issue between
`useEffect` and ref assignment
3. **Maintains Functionality**: Preserves all existing tab navigation
logic (position-based sorting, modal focus trapping, etc.)
4. **Clean Architecture**: Separates event listener attachment logic
into a reusable callback

## Testing

-  Tab navigation now works correctly in Fixed Layout applications
-  Maintains proper top-to-bottom, left-to-right tab order
-  Modal focus trapping continues to work
-  Auto Layout behavior unchanged (tab navigation disabled as intended)
-  No regressions in existing functionality

## Files Changed

- `app/client/src/utils/hooks/useWidgetFocus/useWidgetFocus.tsx` - Fixed
event listener timing
- `app/client/src/utils/hooks/useWidgetFocus/handleTab.ts` - Cleaned up
(no functional changes)
- `app/client/src/utils/hooks/useWidgetFocus/tabbable.ts` - Cleaned up
(no functional changes)

## Automation

/ok-to-test tags="@tag.Widget"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/18034264649>
> Commit: ab9af8404302eb19c243dea583160bc9e74f33aa
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=18034264649&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Widget`
> Spec:
> <hr>Fri, 26 Sep 2025 11:09:55 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
  * Improved reliability of focusing widgets on click.
  * More consistent Tab key navigation across widgets.
  * Prevents unintended focus behavior in non–auto-layout mode.

* **Refactor**
* Streamlined event listener management for focus and keyboard
interactions, improving stability and reducing potential memory leaks.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Jacques Ikot 2025-09-29 01:14:08 +01:00 committed by GitHub
parent 180af275b5
commit 77005b5798
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -1,52 +1,70 @@
import { useCallback, useRef } from "react";
import { useSelector } from "react-redux";
import { useCallback, useEffect, useRef } from "react";
import { getIsAutoLayout } from "selectors/canvasSelectors";
import { handleTab } from "./handleTab";
import { CANVAS_WIDGET } from "./tabbable";
import { getIsAutoLayout } from "selectors/canvasSelectors";
function useWidgetFocus(): (instance: HTMLElement | null) => void {
const ref = useRef<HTMLElement | null>();
const isAutoLayout = useSelector(getIsAutoLayout);
// This is a callback that will be called when the ref is set
const setRef = useCallback((node: HTMLElement | null) => {
if (node === null) return;
if (ref.current === node) return;
ref.current = node;
return ref;
}, []);
useEffect(() => {
if (isAutoLayout) return;
if (!ref.current) return;
const handleKeyDown = (event: KeyboardEvent) => {
if (event.key === "Tab") handleTab(event);
};
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const handleClick = (event: any) => {
const target = event.target as HTMLElement;
if (target.matches(CANVAS_WIDGET)) {
target.focus();
const attachEventListeners = useCallback(
(element: HTMLElement) => {
if (isAutoLayout) {
return () => {}; // Return empty cleanup function
}
};
ref.current.addEventListener("keydown", handleKeyDown);
ref.current.addEventListener("click", handleClick);
const handleKeyDown = (event: KeyboardEvent) => {
if (event.key === "Tab") {
handleTab(event);
}
};
return () => {
ref?.current && ref.current.removeEventListener("keydown", handleKeyDown);
ref?.current && ref.current.removeEventListener("click", handleClick);
};
}, []);
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const handleClick = (event: any) => {
const target = event.target as HTMLElement;
if (target.matches(CANVAS_WIDGET)) {
target.focus();
}
};
element.addEventListener("keydown", handleKeyDown);
element.addEventListener("click", handleClick);
// Return cleanup function
return () => {
element.removeEventListener("keydown", handleKeyDown);
element.removeEventListener("click", handleClick);
};
},
[isAutoLayout],
);
// This is a callback that will be called when the ref is set
const setRef = useCallback(
(node: HTMLElement | null) => {
if (node === null) {
ref.current = null;
return;
}
if (ref.current === node) {
return;
}
ref.current = node;
// Attach event listeners immediately when ref is set
attachEventListeners(node);
return ref;
},
[attachEventListeners],
);
return setRef;
}