feat(apollo-wind): add reusable variable picker - #1020
Conversation
|
Apollo Coded App preview deployments are running.
|
Dependency License Review
License distribution
Excluded packages
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a reusable VariablePicker component in apollo-wind (searchable hierarchical variable insertion UI) and wires it into existing variable-insertion surfaces (LockableValueField + apollo-react canvas editors/stories), while also refining some canvas alignment guide rendering and Storybook sidebar ordering.
Changes:
- Added and publicly exported
VariablePicker(+ tests + Storybook docs) in@uipath/apollo-wind. - Integrated the picker into LockableValueField and apollo-react ExpressionField / NodePropertyPanel stories to standardize insertion UX.
- Updated alignment guide rendering to show all matching anchors at the snapped position, plus Storybook ordering adjustments.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/apollo-wind/src/index.ts | Exports VariablePicker and its public types from the package entrypoint. |
| packages/apollo-wind/src/components/ui/variable-picker/variable-picker.tsx | Implements the new hierarchical/searchable picker component. |
| packages/apollo-wind/src/components/ui/variable-picker/variable-picker.test.tsx | Adds unit tests for open/selection/search/empty-state behavior. |
| packages/apollo-wind/src/components/ui/variable-picker/variable-picker.stories.tsx | Adds Storybook documentation + usage examples (custom trigger, controlled, disabled, empty). |
| packages/apollo-wind/src/components/ui/variable-picker/index.ts | Barrel export for the new component and types. |
| packages/apollo-wind/src/components/ui/lockable-value-field/lockable-value-field.test.tsx | Updates the insertion test to match the picker’s option role. |
| packages/apollo-wind/src/components/ui/lockable-value-field/components/field-header.tsx | Replaces the dropdown-based insert UI with VariablePicker. |
| packages/apollo-wind/.storybook/preview.tsx | Adjusts Storybook sort order to place Variable Picker after Lockable Value Field. |
| packages/apollo-react/src/canvas/components/NodePropertyPanel/NodePropertyPanel.stories.tsx | Uses VariablePicker in editor toolbars and aligns IO tree row actions (wrap/copy/add-variable/toasts). |
| packages/apollo-react/src/canvas/components/NodePropertyPanel/ExpressionField.tsx | Adds variable picker support via new variables / onInsertVariable props. |
| packages/apollo-react/src/canvas/components/AlignmentGuides/useAlignmentGuides.ts | Updates guide rendering to show all exact matches at the snapped position. |
| packages/apollo-react/src/canvas/components/AlignmentGuides/useAlignmentGuides.test.ts | Updates/adds tests for multiple guide rendering and unique IDs. |
| packages/apollo-react/src/canvas/components/AlignmentGuides/AlignmentGuides.stories.tsx | Tweaks multiselect story interactions (shift-click selection behavior). |
| apps/storybook/.storybook/preview.tsx | Mirrors Storybook sort order adjustments in the root Storybook app. |
Suppressed comments (1)
packages/apollo-react/src/canvas/components/NodePropertyPanel/NodePropertyPanel.stories.tsx:1889
JSON.stringify(value)is evaluated beforeclipboard.writeText(...)and can throw (e.g., circular structures, BigInt). That would crash the Storybook interaction. Consider guarding the stringify and falling back toString(value)on failure.
const copyValue = (node: OutputNode) => {
const value = editedValues[node.path] ?? node.value;
navigator.clipboard
?.writeText(typeof value === 'string' ? value : JSON.stringify(value))
?.then(() => {
📊 Coverage + size by packagePer-package coverage and bundle size on this PR. New-line coverage = of the source lines this PR adds or changes, the % hit by tests.
"Coverage" is each package's own |
Storybook visual diffBaseline is the deployed main Storybook, so changes merged to main after this branch was last updated can also appear here. Logs Updated (PT): Aug 06, 2026, 08:07:44 AM |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/apollo-wind/src/components/ui/variable-picker/variable-picker.tsx:166
expandedIdsis only initialized fromitemsonce. Ifitemsis empty initially (e.g., async load) or later replaced, opening the picker can show everything collapsed because the default-expanded group is never re-derived from the latestitems. Consider re-seeding expansion when the popover opens if the current expanded IDs don’t match the current root items.
const setOpen = (nextOpen: boolean) => {
if (controlledOpen === undefined) setUncontrolledOpen(nextOpen);
if (!nextOpen) setQuery('');
onOpenChange?.(nextOpen);
};
packages/apollo-react/src/canvas/components/NodePropertyPanel/NodePropertyPanel.stories.tsx:1302
- This file contains
—(em dash with spaces) in Storybook content, which is disallowed by repo convention (seeCLAUDE.md:5-13). Please replace it with a period/colon/comma.
// Inline expression inputs — one per case, no code editor panel.
packages/apollo-wind/src/components/ui/variable-picker/variable-picker.tsx:69
- Using
toLocaleLowerCase()for search normalization can cause surprising matches in some locales (it’s locale-sensitive). For consistent search behavior across environments, prefertoLowerCase()here.
function matchesItem(item: VariablePickerItem, query: string): boolean {
if (!query) return true;
const normalizedQuery = query.toLocaleLowerCase();
return (
item.label.toLocaleLowerCase().includes(normalizedQuery) ||
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (4)
packages/apollo-wind/.storybook/preview.tsx:87
- The Storybook
storySort.orderstructure looks malformed here:UiPathis listed as a standalone entry and the following array of children is not nested under it. This can cause Storybook to treat the child list as a separate top-level group instead of ordering underUiPath.
'Overlays',
'UiPath',
[
'Lockable Value Field',
'Variable Picker',
packages/apollo-wind/src/components/ui/variable-picker/variable-picker.tsx:166
expandedIdsis initialized fromitemsonly on the first render. Ifitemsis initially empty (common for async data) and later populated, opening the picker won't expand the first group as intended. Consider seedingexpandedIdsthe first time the popover opens when it’s still empty.
const setOpen = (nextOpen: boolean) => {
if (controlledOpen === undefined) setUncontrolledOpen(nextOpen);
if (!nextOpen) setQuery('');
onOpenChange?.(nextOpen);
};
apps/storybook/.storybook/preview.tsx:172
- The Storybook
storySort.orderstructure appears malformed:UiPathis a standalone entry and the subsequent array of story names is not nested beneath it. Storybook ordering expects nested arrays like['UiPath', [...]]for hierarchical groups.
'Overlays',
'UiPath',
[
'Lockable Value Field',
'Variable Picker',
packages/apollo-wind/src/components/ui/variable-picker/variable-picker.tsx:67
- Using
toLocaleLowerCase()for search matching makes results locale-dependent (e.g., Turkish locale casing) and can yield inconsistent filtering across environments. Prefer deterministictoLowerCase()for UI search normalization.
const normalizedQuery = query.toLocaleLowerCase();
return (
item.label.toLocaleLowerCase().includes(normalizedQuery) ||
item.value?.toLocaleLowerCase().includes(normalizedQuery) === true ||
item.children?.some((child) => matchesItem(child, query)) === true
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/apollo-react/src/canvas/components/NodePropertyPanel/NodePropertyPanel.stories.tsx:1304
- Story files should avoid using em dashes (
—) in documentation copy. Replace this em dash with punctuation that matches the repo guideline (e.g. a period or colon).
// ============================================================================
// Input Editor
// Inline expression inputs — one per case, no code editor panel.
// ============================================================================
packages/apollo-wind/src/components/ui/variable-picker/variable-picker.tsx:69
matchesItemusestoLocaleLowerCase(), which makes search behavior locale-dependent (e.g. Turkish-I edge cases). For deterministic, locale-agnostic matching, prefertoLowerCase()for both the query and item fields.
function matchesItem(item: VariablePickerItem, query: string): boolean {
if (!query) return true;
const normalizedQuery = query.toLocaleLowerCase();
return (
item.label.toLocaleLowerCase().includes(normalizedQuery) ||
item.value?.toLocaleLowerCase().includes(normalizedQuery) === true ||
item.children?.some((child) => matchesItem(child, query)) === true
);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (4)
packages/apollo-react/src/canvas/components/NodePropertyPanel/NodePropertyPanel.stories.tsx:1303
- Avoid using em dashes in Storybook stories content/comments so the docs copy stays consistent with repo conventions.
// Inline expression inputs — one per case, no code editor panel.
packages/apollo-react/src/canvas/components/NodePropertyPanel/NodePropertyPanel.stories.tsx:4686
- The doc comment above
useFileNodeActionsno longer matches the behavior after adding the Add variable action (non-file rows now return actions; read-only file rows return Add variable). It also uses em dashes, which we avoid in Storybook docs text.
*/
function useFileNodeActions(): {
nodeActions: NodeActionsResolver;
preview: PreviewedFile | null;
clearPreview: () => void;
packages/apollo-wind/src/components/ui/variable-picker/variable-picker.tsx:69
matchesItemusestoLocaleLowerCase(), which can produce locale-dependent casing behavior. For search matching,toLowerCase()is typically more predictable and consistent across environments.
const normalizedQuery = query.toLocaleLowerCase();
return (
item.label.toLocaleLowerCase().includes(normalizedQuery) ||
item.value?.toLocaleLowerCase().includes(normalizedQuery) === true ||
item.children?.some((child) => matchesItem(child, query)) === true
packages/apollo-wind/src/components/ui/variable-picker/variable-picker.tsx:98
CommandItemrenders a cmdk option (role="option"), andaria-expandedis not a valid ARIA state for that role. Consider using adata-*attribute for styling, or switching to true tree semantics if you need expanded/collapsed announcements.
aria-expanded={hasChildren ? expanded : undefined}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/apollo-react/src/canvas/components/NodePropertyPanel/NodePropertyPanel.stories.tsx:1303
- Story source comment uses an em dash (" — "). The repo guideline for
.stories.tsxcontent avoids em dashes because the source is user-visible in Storybook docs; please replace with a period/colon instead.
// Inline expression inputs — one per case, no code editor panel.
4dfc99f to
fec447e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/apollo-wind/src/components/ui/variable-picker/variable-picker.tsx:51
childrenis passed as thePopoverTriggerchild withasChild, which requires a single React element. Typing this asReactNodeallows strings/fragments/arrays that will crash at runtime when Radix tries to clone the child. Narrow the type so invalid triggers are caught by TypeScript.
children?: ReactNode;
packages/apollo-wind/src/components/ui/variable-picker/variable-picker.tsx:95
cmdkuses theCommandItemvalueas the item's identity for selection/active state. Building it fromlabel/valuecan collide even whenidis unique, which can cause incorrect highlighting/selection when two variables share the same label or path segment. Use the guaranteed-uniqueidinstead.
value={`${item.label} ${item.value ?? ''}`}
What this adds
Reusable Insert Variable component
VariablePickerto Apollo WindVariablePickerandVariablePickerItemfor consumersAdd Variable tree action
field2,field3, etc.), expands its parent, and focuses the blank value•••menu•••trigger visible while open and positions the menu beside the cursor with the first action immediately accessibleAligned Node Property Panel examples
Validation