[ENG-1416] Fix 'create node modal' focus on input box on load#723
[ENG-1416] Fix 'create node modal' focus on input box on load#723trangdoan982 wants to merge 6 commits intomainfrom
Conversation
…Enter selection - Add autoFocus to content input field to focus on dialog open - Refocus input after selection to maintain keyboard workflow - Prevents focus from jumping to Node Type dropdown unexpectedly Fixes ENG-1316 Co-authored-by: doantranghp2000 <doantranghp2000@gmail.com>
|
Cursor Agent can help with this pull request. Just |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
mdroidian
left a comment
There was a problem hiding this comment.
This implementation introduces multiple competing focus mechanisms (Dialog focus + two useEffects + component splitting). That’s fragile because re-renders, portal mount timing, and transition/animation frames can cause focus to bounce or re-trigger.
I’d like us to:
- Use Blueprint-native focus first: ensure the intended input has
autoFocus, and confirm the Dialog’s focus behavior is configured correctly (enforceFocusetc.). - If we still need JS, keep it to one fallback effect scoped to
isOpenthat runs once per open (e.g.,if (isOpen && !hasFocusedRef.current) focus();). No second component solely for focus.
Worth asking yourself: was this present in the prior modal? If not, identify what changed and why Blueprint’s default focus path no longer works. Also check other dialogs in this repo that have this problem and look for an established pattern.
mdroidian
left a comment
There was a problem hiding this comment.
https://www.loom.com/share/37020a3f127747e185a54449008fb093
diff --git a/apps/roam/src/components/FuzzySelectInput.tsx b/apps/roam/src/components/FuzzySelectInput.tsx
index 3d8a6629..efea2fa9 100644
--- a/apps/roam/src/components/FuzzySelectInput.tsx
+++ b/apps/roam/src/components/FuzzySelectInput.tsx
@@ -200,7 +200,6 @@ const FuzzySelectInput = <T extends Result = Result>({
<InputGroup
fill
className="w-full"
- disabled={disabled}
value={query}
onChange={(e) => setQuery(e.target.value)}
onKeyDown={handleKeyDown}
diff --git a/apps/roam/src/components/ModifyNodeDialog.tsx b/apps/roam/src/components/ModifyNodeDialog.tsx
index d949717e..4c04e080 100644
--- a/apps/roam/src/components/ModifyNodeDialog.tsx
+++ b/apps/roam/src/components/ModifyNodeDialog.tsx
@@ -539,6 +539,7 @@ const ModifyNodeDialog = ({
disabled={loading}
mode={mode}
initialUid={content.uid}
+ autoFocus
/>
</div>
@@ -555,6 +556,7 @@ const ModifyNodeDialog = ({
mode={"create"}
initialUid={referencedNodeValue.uid}
initialIsLocked={isReferencedNodeLocked}
+ autoFocus={false}
/>
</div>
)}
|
It looks like the PR 723 was linked to the wrong linear issue. 723 is fixing FEE-695 which did not yet have an ENG ticket. I went ahead and created the ENG ticket, changed the title of this PR, and updated the linear links. |
| } | ||
| }, [activeIndex, isOpen]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
on Canvas in "create" mode the autofocus doesn't get applied.
https://www.loom.com/share/a092cd458bef479daefce722473e0f0f
Fixes focus jumping to the node type dropdown after selecting a node in ModifyNodeDialog.
This ensures the content input field remains focused, both on dialog open and after selection, for a smoother keyboard-first workflow.
UPDATE from @mdroidian : PR title and linear task linked changed. ENG-1416 is the correct task. ENG-1316 is a different issue.