feat(Suggestions): Add lightbulb button and double-click to open suggestions#98
Merged
lordrip merged 2 commits intoKaotoIO:1.xfrom Mar 17, 2026
Merged
Conversation
Lift the local visibility state (isVisible/setIsVisible) from the
useSuggestions hook into SuggestionContext so only one suggestion menu
can be open at a time across all fields.
- Add currentOpenMenu/setCurrentOpenMenu to SuggestionContext
- Replace local useState with context-driven visibility using unique menuId (useId)
- Change useSuggestions return type from ReactNode to { suggestionsMenu, openSuggestions }
- Update all field consumers and tests to use the new return shape
📝 WalkthroughWalkthroughRefactors the suggestions system: Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Field as Form Field
participant Button as SuggestionsButton
participant Hook as useSuggestions
participant Context as SuggestionContext
participant Menu as SuggestionsMenu
User->>Field: Type or press Ctrl+Space / dblclick
Field->>Hook: request suggestions for prop
Hook->>Context: read currentOpenMenu
Hook->>Context: setCurrentOpenMenu(menuId)
Context-->>Hook: currentOpenMenu updated
Hook-->>Field: return { suggestionsMenu, openSuggestions }
Field->>Menu: render suggestionsMenu (when currentOpenMenu matches)
alt User clicks SuggestionsButton
User->>Button: click
Button->>Hook: openSuggestions()
Hook->>Context: setCurrentOpenMenu(menuId)
Hook->>Menu: focus first item
end
Menu-->>User: shows focused suggestion list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip You can customize the tone of the review comments and chat replies.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/form/KaotoForm.scss (1)
45-55: Consider keyboard accessibility for the suggestions button.The button is hidden by default (
display: none) and only becomes visible on parent hover or when the button itself is focused. However, a hidden element cannot receive keyboard focus, so users navigating via keyboard cannot reach the button unless they first hover over the parent with a mouse.Consider adding a
:focus-withinselector on the parent containers to show the button when any child (like the input) has focus:♻️ Suggested enhancement for keyboard accessibility
/* stylelint-disable-next-line selector-class-pattern */ .pf-v6-c-text-input-group:hover &__suggestions-button, /* stylelint-disable-next-line selector-class-pattern */ .pf-v6-c-input-group:hover &__suggestions-button, + /* stylelint-disable-next-line selector-class-pattern */ + .pf-v6-c-text-input-group:focus-within &__suggestions-button, + /* stylelint-disable-next-line selector-class-pattern */ + .pf-v6-c-input-group:focus-within &__suggestions-button, &__suggestions-button:focus { display: inline-flex; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/KaotoForm.scss` around lines 45 - 55, The suggestions button is hidden by default so keyboard users can't focus it; update the SASS rules for &__suggestions-button so it becomes visible not only on parent hover but also when the parent has focus-within (e.g., add selectors like .pf-v6-c-text-input-group:focus-within &__suggestions-button and .pf-v6-c-input-group:focus-within &__suggestions-button), ensuring the button can be reached via keyboard focus from the input; confirm &__suggestions-button remains a focusable element (not aria-hidden or display:none when focused).src/form/hooks/suggestions.test.tsx (1)
63-67: Add a direct regression test for the double-click trigger path.The PR introduces
dblclickopening behavior, but this suite still only asserts keyboard-open flows.✅ Suggested test addition
+ it('should show suggestions menu when input is double-clicked', async () => { + const result = renderWithContext(<TestComponent />); + + await act(async () => { + fireEvent.doubleClick(result.getByTestId('test-input')); + }); + + await waitFor(() => { + expect(result.getByTestId('suggestions-menu')).toBeInTheDocument(); + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/hooks/suggestions.test.tsx` around lines 63 - 67, Add a regression test that covers the new double-click open path: using the existing renderWithContext helper to render the component wrapped by StatefulSuggestionProvider (with getProvidersMock), simulate a double-click on the input/trigger element (use fireEvent.doubleClick or userEvent.dblClick against the same element used in other keyboard tests), then assert that the suggestion list appears (e.g., query/getByRole for listbox or the expected suggestion items) and that the provider was invoked as expected; add this alongside the existing keyboard-open tests in suggestions.test.tsx to ensure dblclick behavior is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/form/hooks/suggestions.tsx`:
- Around line 154-166: The openSuggestions callback must no-op for
non-interactive inputs; add an early return that checks the bound input's
interactive state (e.g., a prop or ref such as disabled, readOnly, or
inputRef.current?.disabled/inputRef.current?.readOnly) before calling
setSearchValue or setCurrentOpenMenu; keep the existing focus/scroll logic but
only execute it when the input is interactive and when newValue is truthy (use
the existing menuId, setCurrentOpenMenu, firstElementRef references).
---
Nitpick comments:
In `@src/form/hooks/suggestions.test.tsx`:
- Around line 63-67: Add a regression test that covers the new double-click open
path: using the existing renderWithContext helper to render the component
wrapped by StatefulSuggestionProvider (with getProvidersMock), simulate a
double-click on the input/trigger element (use fireEvent.doubleClick or
userEvent.dblClick against the same element used in other keyboard tests), then
assert that the suggestion list appears (e.g., query/getByRole for listbox or
the expected suggestion items) and that the provider was invoked as expected;
add this alongside the existing keyboard-open tests in suggestions.test.tsx to
ensure dblclick behavior is exercised.
In `@src/form/KaotoForm.scss`:
- Around line 45-55: The suggestions button is hidden by default so keyboard
users can't focus it; update the SASS rules for &__suggestions-button so it
becomes visible not only on parent hover but also when the parent has
focus-within (e.g., add selectors like .pf-v6-c-text-input-group:focus-within
&__suggestions-button and .pf-v6-c-input-group:focus-within
&__suggestions-button), ensuring the button can be reached via keyboard focus
from the input; confirm &__suggestions-button remains a focusable element (not
aria-hidden or display:none when focused).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 127d7e50-30d5-4d38-ae35-adad943bf066
⛔ Files ignored due to path filters (4)
src/form/__snapshots__/KaotoForm.test.tsx.snapis excluded by!**/*.snapsrc/form/fields/__snapshots__/PasswordField.test.tsx.snapis excluded by!**/*.snapsrc/form/fields/__snapshots__/StringField.test.tsx.snapis excluded by!**/*.snapsrc/form/fields/__snapshots__/TextAreaField.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (14)
src/form/KaotoForm.scsssrc/form/KeyValue/KeyValueField.test.tsxsrc/form/KeyValue/KeyValueField.tsxsrc/form/fields/FieldActions.test.tsxsrc/form/fields/PasswordField.test.tsxsrc/form/fields/PasswordField.tsxsrc/form/fields/StringField.test.tsxsrc/form/fields/StringField.tsxsrc/form/fields/SuggestionsButton.tsxsrc/form/fields/TextAreaField.test.tsxsrc/form/fields/TextAreaField.tsxsrc/form/hooks/suggestions.test.tsxsrc/form/hooks/suggestions.tsxsrc/form/providers/SuggestionRegistryProvider.tsx
…estions menu - Add SuggestionsButton component with lightbulb icon, shown on hover/focus - Add double-click event listener on input fields to toggle suggestions - Add CSS rules to show/hide the button on parent hover - Update StringField, PasswordField, and TextAreaField to include the button
mmelko
force-pushed
the
feat/suggetions-button
branch
from
396b92f to
8c01bb1
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/form/hooks/suggestions.tsx (1)
59-66:⚠️ Potential issue | 🟡 MinorMake all “open suggestions” triggers follow the same open semantics.
Line 59 opens unconditionally, while Line 160 toggles (
menuId↔null). Since this callback is used by the lightbulb and double-click “open” actions, toggling can close the menu unexpectedly. Prefer an idempotent open path and reuse it for keyboard/button/dblclick.Also applies to: 154-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/hooks/suggestions.tsx` around lines 59 - 66, The keyboard/open handler opens the suggestions unconditionally (setCurrentOpenMenu(menuId)) while the other handler toggles (menuId ↔ null), causing double-click and lightbulb actions to sometimes close the menu; make all "open suggestions" triggers idempotent by extracting/using a single open function that calls setSearchValue(''), setCurrentOpenMenu(menuId) (never toggling to null), and focuses/scrolls firstElementRef.current inside requestAnimationFrame, then replace the toggle logic in the double-click/lightbulb handler (the code around the current toggle at lines ~154-169) to call that same open function so every trigger shares the same open semantics.
🧹 Nitpick comments (1)
src/form/fields/SuggestionsButton.tsx (1)
5-8: Support a disabled state on the trigger button.The component currently has no
isDisabledprop, which makes it harder for consuming fields to render a non-interactive trigger when the input is disabled. Adding it here keeps behavior explicit and consistent across all field types.Proposed diff
interface SuggestionsButtonProps { propName: string; onClick: () => void; + isDisabled?: boolean; } -export const SuggestionsButton: FunctionComponent<SuggestionsButtonProps> = ({ propName, onClick }) => ( +export const SuggestionsButton: FunctionComponent<SuggestionsButtonProps> = ({ propName, onClick, isDisabled }) => ( <Button variant="plain" className="kaoto-form__suggestions-button" data-testid={`${propName}__open-suggestions-button`} onClick={onClick} + isDisabled={isDisabled} aria-label="Open suggestions" title="Open suggestions (Ctrl+Space)" icon={<LightbulbIcon />} /> );Also applies to: 10-19
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/fields/SuggestionsButton.tsx` around lines 5 - 8, The SuggestionsButtonProps interface lacks an isDisabled flag so consumers can't render a non-interactive trigger; add isDisabled?: boolean to SuggestionsButtonProps and update the SuggestionsButton component (props propName, onClick) to accept and apply that prop to the trigger element (set the button's disabled attribute and any relevant ARIA/state styling) and propagate the prop where SuggestionsButton is instantiated so disabled inputs render a non-interactive trigger.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/form/hooks/suggestions.tsx`:
- Around line 59-66: The keyboard/open handler opens the suggestions
unconditionally (setCurrentOpenMenu(menuId)) while the other handler toggles
(menuId ↔ null), causing double-click and lightbulb actions to sometimes close
the menu; make all "open suggestions" triggers idempotent by extracting/using a
single open function that calls setSearchValue(''), setCurrentOpenMenu(menuId)
(never toggling to null), and focuses/scrolls firstElementRef.current inside
requestAnimationFrame, then replace the toggle logic in the
double-click/lightbulb handler (the code around the current toggle at lines
~154-169) to call that same open function so every trigger shares the same open
semantics.
---
Nitpick comments:
In `@src/form/fields/SuggestionsButton.tsx`:
- Around line 5-8: The SuggestionsButtonProps interface lacks an isDisabled flag
so consumers can't render a non-interactive trigger; add isDisabled?: boolean to
SuggestionsButtonProps and update the SuggestionsButton component (props
propName, onClick) to accept and apply that prop to the trigger element (set the
button's disabled attribute and any relevant ARIA/state styling) and propagate
the prop where SuggestionsButton is instantiated so disabled inputs render a
non-interactive trigger.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 851a84cc-7722-47f6-91c5-1ee88715a740
⛔ Files ignored due to path filters (4)
src/form/__snapshots__/KaotoForm.test.tsx.snapis excluded by!**/*.snapsrc/form/fields/__snapshots__/PasswordField.test.tsx.snapis excluded by!**/*.snapsrc/form/fields/__snapshots__/StringField.test.tsx.snapis excluded by!**/*.snapsrc/form/fields/__snapshots__/TextAreaField.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
src/form/KaotoForm.scsssrc/form/fields/FieldActions.test.tsxsrc/form/fields/PasswordField.tsxsrc/form/fields/StringField.tsxsrc/form/fields/SuggestionsButton.tsxsrc/form/fields/TextAreaField.tsxsrc/form/hooks/suggestions.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/form/KaotoForm.scss
- src/form/fields/FieldActions.test.tsx
Contributor
Author
|
video was updated |
lordrip
approved these changes
Mar 17, 2026
This was referenced Mar 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Centralize suggestion menu visibility state in SuggestionContext so only one suggestion menu can be open at a time across all fields
Add a lightbulb button to StringField, PasswordField, and TextAreaField that opens the suggestions menu on click (visible on hover/focus)
Add double-click handler on input fields as an alternative way to open suggestions
Change useSuggestions return type from ReactNode to { suggestionsMenu, openSuggestions } to support the new interaction methods
Screen.Recording.2026-03-17.at.17.22.17.mov
Summary by CodeRabbit