Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

import { useEffect, useRef, useState, useCallback, KeyboardEvent, FC } from 'react';
import { WidgetProps } from '@rjsf/utils';
import { InputAdornment, TextField, Tooltip } from '@mui/material';
import { IconButton, InputAdornment, TextField, Tooltip } from '@mui/material';
import { Delete } from '@mui/icons-material';
import { useTranslation } from 'react-i18next';

Expand Down Expand Up @@ -110,6 +110,18 @@ const ApiKeyWidget: FC<WidgetProps> = ({ id, value, label, required, autofocus,
[autocomplete, onEnvVarSelect],
);

// Open the full variable list from the picker button (no `${` typing needed).
const handleOpenPicker = useCallback(() => {
const el = inputRef.current;
if (!el) return;
el.focus();
autocomplete.openAll(el, el.selectionStart ?? String(tempValue ?? '').length);
}, [autocomplete, tempValue]);

// Only offer the picker while the field is editable (an existing key is masked
// + read-only; the user clears it via the trash button before entering a new one).
const showVarPicker = envKeys.length > 0 && !disabled && !readonly && !maskApiKey;

// When in masked mode, scroll the input to the end so the visible trailing characters are shown
useEffect(() => {
if (inputRef.current && maskApiKey) {
Expand Down Expand Up @@ -147,7 +159,7 @@ const ApiKeyWidget: FC<WidgetProps> = ({ id, value, label, required, autofocus,
slotProps={{
input: {
readOnly: maskApiKey || readonly,
endAdornment: maskApiKey && !readonly && (
endAdornment: maskApiKey && !readonly ? (
<InputAdornment position="end">
<Tooltip title={t('form.apiKeyRemoveTooltip')}>
<Delete
Expand All @@ -167,7 +179,21 @@ const ApiKeyWidget: FC<WidgetProps> = ({ id, value, label, required, autofocus,
/>
</Tooltip>
</InputAdornment>
),
) : showVarPicker ? (
<InputAdornment position="end">
<IconButton
size="small"
edge="end"
tabIndex={-1}
aria-label="Insert variable"
title="Insert variable"
onClick={handleOpenPicker}
sx={{ fontSize: 13, fontFamily: 'var(--rr-font-family-widget, monospace)', color: 'var(--rr-text-secondary)', px: 0.5 }}
>
{'${}'}
</IconButton>
</InputAdornment>
) : undefined,
},
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

import { ChangeEvent, FocusEvent, useState, useEffect, useCallback, useRef, KeyboardEvent } from 'react';
import TextField, { TextFieldProps } from '@mui/material/TextField';
import InputAdornment from '@mui/material/InputAdornment';
import IconButton from '@mui/material/IconButton';
import { ariaDescribedByIds, BaseInputTemplateProps, examplesId, getInputProps, labelValue, FormContextType, RJSFSchema, StrictRJSFSchema } from '@rjsf/utils';

import { useEnvVarAutocomplete } from '../hooks/useEnvVarAutocomplete';
Expand Down Expand Up @@ -125,6 +127,16 @@ export default function BaseInputTemplate<
[autocomplete, onEnvVarSelect],
);

// Open the full variable list from the picker button (no `${` typing needed).
const handleOpenPicker = useCallback(() => {
const el = inputRef.current;
if (!el) return;
el.focus();
autocomplete.openAll(el, el.selectionStart ?? String(controlledValue ?? '').length);
}, [autocomplete, controlledValue]);

const showVarPicker = envKeys.length > 0 && !disabled && !readonly;

// Sync controlled value when the form re-renders with a new prop value (e.g., reset or external update)
useEffect(() => {
if (value !== undefined && value !== null) {
Expand Down Expand Up @@ -195,6 +207,27 @@ export default function BaseInputTemplate<
inputRef={inputRef}
InputLabelProps={DisplayInputLabelProps}
{...(textFieldProps as TextFieldProps)}
InputProps={{
...(textFieldProps as TextFieldProps).InputProps,
endAdornment: showVarPicker ? (
<InputAdornment position="end">
<IconButton
size="small"
edge="end"
tabIndex={-1}
aria-label="Insert variable"
title="Insert variable"
onClick={handleOpenPicker}
Comment on lines +214 to +220

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Shared root cause: picker affordance is excluded from keyboard navigation via tabIndex={-1}. This defeats the discoverability fix for keyboard-only users in both widgets.

  • packages/shared-ui/src/components/canvas/components/rjsf-widgets/base-input-template/BaseInputTemplate.tsx#L214-L220: remove tabIndex={-1} from the picker IconButton so it participates in tab order.
  • packages/shared-ui/src/components/canvas/components/rjsf-widgets/textarea-widget/TextareaWidget.tsx#L110-L116: apply the same change to the textarea picker IconButton.
📍 Affects 2 files
  • packages/shared-ui/src/components/canvas/components/rjsf-widgets/base-input-template/BaseInputTemplate.tsx#L214-L220 (this comment)
  • packages/shared-ui/src/components/canvas/components/rjsf-widgets/textarea-widget/TextareaWidget.tsx#L110-L116
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/shared-ui/src/components/canvas/components/rjsf-widgets/base-input-template/BaseInputTemplate.tsx`
around lines 214 - 220, The picker IconButton components are set to
tabIndex={-1}, which removes them from keyboard navigation and makes them
inaccessible to keyboard-only users. Remove the tabIndex={-1} attribute from the
picker IconButton in
packages/shared-ui/src/components/canvas/components/rjsf-widgets/base-input-template/BaseInputTemplate.tsx
(lines 214-220) with the aria-label "Insert variable", and apply the same fix by
removing tabIndex={-1} from the picker IconButton in
packages/shared-ui/src/components/canvas/components/rjsf-widgets/textarea-widget/TextareaWidget.tsx
(lines 110-116). This will allow both picker buttons to participate in the
normal tab order and be discoverable to keyboard users.

sx={{ fontSize: 13, fontFamily: 'var(--rr-font-family-widget, monospace)', color: 'var(--rr-text-secondary)', px: 0.5 }}
>
{'${}'}
</IconButton>
{(textFieldProps as TextFieldProps).InputProps?.endAdornment}
</InputAdornment>
) : (
(textFieldProps as TextFieldProps).InputProps?.endAdornment
),
}}
sx={{
...sx,
'& input[type="number"]::-webkit-outer-spin-button, & input[type="number"]::-webkit-inner-spin-button': {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ export interface UseEnvVarAutocompleteResult {
anchorEl: HTMLElement | null;
/** Called on every input change to detect the `${` trigger. Pass the value and cursor position directly from the event target. */
handleInputChange: (value: string, cursorPos: number, anchorElement: HTMLElement | null) => void;
/** Opens the full list of available keys (no `${` needed), inserting at `cursorPos` on select. Used by the explicit picker button. */
openAll: (anchorElement: HTMLElement | null, cursorPos: number) => void;
/** Called when the user selects a suggestion. Returns the new field value. */
handleSelect: (key: string, currentValue: string, inputEl: HTMLInputElement | HTMLTextAreaElement | null) => string;
/** Dismisses the popover. */
Expand Down Expand Up @@ -110,6 +112,21 @@ export function useEnvVarAutocomplete(envKeys: string[]): UseEnvVarAutocompleteR
[],
);

const openAll = useCallback(
(anchorElement: HTMLElement | null, cursorPos: number) => {
if (!anchorElement || !envKeys.length) return;
// No `${` was typed — insert at the current cursor position. handleSelect
// replaces from triggerStart to the cursor, so a zero-width range there
// just inserts `${KEY}` without clobbering surrounding text.
triggerStartRef.current = cursorPos;
setSuggestions(envKeys);
setAnchorEl(anchorElement);
setHighlightedIndex(0);
setIsOpen(true);
},
[envKeys],
);
Comment on lines +115 to +128

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add focused tests for the new openAll insertion path.

openAll introduces a second entry path into selection/insertion semantics. Add hook/widget tests that validate cursor-position insertion and popover open state so regressions don’t slip in.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/shared-ui/src/components/canvas/components/rjsf-widgets/hooks/useEnvVarAutocomplete.ts`
around lines 115 - 128, The new openAll callback function in the
useEnvVarAutocomplete hook introduces a second code path for handling
environment variable autocomplete insertion (cursor-position based insertion
without requiring the ${ prefix). Add test cases in the hook tests or widget
tests that specifically validate this openAll behavior, including tests that
confirm the triggerStartRef is correctly set to the cursor position, that
setSuggestions populates with envKeys, that setIsOpen sets the popover to open
state, and that setHighlightedIndex initializes to 0. These tests should cover
the insertion semantics when openAll is called with a valid anchorElement and
cursorPos to ensure this alternative entry path works correctly alongside the
existing trigger-based path.


const handleDismiss = useCallback(() => {
setIsOpen(false);
}, []);
Expand All @@ -124,5 +141,5 @@ export function useEnvVarAutocomplete(envKeys: string[]): UseEnvVarAutocompleteR
[suggestions.length],
);

return { isOpen, suggestions, anchorEl, handleInputChange, handleSelect, handleDismiss, highlightedIndex, moveHighlight };
return { isOpen, suggestions, anchorEl, handleInputChange, openAll, handleSelect, handleDismiss, highlightedIndex, moveHighlight };
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import { ChangeEvent, FocusEvent, useState, useEffect, useCallback, useRef, KeyboardEvent, FC } from 'react';
import TextField from '@mui/material/TextField';
import InputAdornment from '@mui/material/InputAdornment';
import IconButton from '@mui/material/IconButton';
import { WidgetProps } from '@rjsf/utils';

import { useEnvVarAutocomplete } from '../hooks/useEnvVarAutocomplete';
Expand Down Expand Up @@ -41,6 +43,16 @@ const TextareaWidget: FC<WidgetProps> = ({ id, value, label, required, autofocus
[autocomplete, controlledValue, onChange, options.emptyValue],
);

// Open the full variable list from the picker button (no `${` typing needed).
const handleOpenPicker = useCallback(() => {
const el = inputRef.current;
if (!el) return;
el.focus();
autocomplete.openAll(el, el.selectionStart ?? String(controlledValue ?? '').length);
}, [autocomplete, controlledValue]);

const showVarPicker = envKeys.length > 0 && !disabled && !readonly;

const handleKeyDown = useCallback(
(e: KeyboardEvent<HTMLTextAreaElement>) => {
if (!autocomplete.isOpen) return;
Expand Down Expand Up @@ -90,6 +102,27 @@ const TextareaWidget: FC<WidgetProps> = ({ id, value, label, required, autofocus
disabled={disabled || readonly}
error={!!rawErrors?.length}
variant="outlined"
InputProps={
showVarPicker
? {
endAdornment: (
<InputAdornment position="end" sx={{ alignItems: 'flex-start', mt: 1 }}>
<IconButton
size="small"
edge="end"
tabIndex={-1}
aria-label="Insert variable"
title="Insert variable"
onClick={handleOpenPicker}
sx={{ fontSize: 13, fontFamily: 'var(--rr-font-family-widget, monospace)', color: 'var(--rr-text-secondary)', px: 0.5 }}
>
{'${}'}
</IconButton>
</InputAdornment>
),
}
: undefined
}
InputLabelProps={{ shrink: true }}
helperText={typeof options?.description === 'string' ? options.description : schema?.description}
/>
Expand Down
Loading