[7123] Search and Browse dialogs should either hide existing selected items or show them as already selected

[jvega190]

craftercms/craftercms#7123

Summary by CodeRabbit

• New Features
  • Image picker now preserves and preselects the current image when opening Browse or Search dialogs.
  • Node selector now preselects existing items when opening Browse or Search dialogs, streamlining multi-item selection.

[coderabbitai]

Walkthrough

Passes optional preselectedPaths through FormsEngine dialogs so ImagePicker and NodeSelector preselect their current values when opening Browse or Search dialogs; helper functions' signatures updated to accept the new option.

Changes

Cohort / File(s)	Summary
Control Components ui/app/src/components/FormsEngine/controls/ImagePicker.tsx, ui/app/src/components/FormsEngine/controls/NodeSelector.tsx	Call showBrowseFilesDialog and showSearchDialog with preselectedPaths derived from current value(s) so dialogs open with existing selections preselected.
Helper Library ui/app/src/components/FormsEngine/lib/controlHelpers.tsx	Added optional preselectedPaths?: string[] to showBrowseFilesDialog and showSearchDialog signatures and forward the field in the dispatch payloads to dialog components.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• [7558] Select All is missing in the multi-select Item-Selector Browse dialog #4416: Adds preselectedPaths handling in BrowseFilesDialog to surface and use preselected items, aligning with the preselection flow introduced here.

Suggested reviewers

• rart

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description only contains a GitHub issue link with no additional context or explanation of the changes made.	Expand the description to include a brief explanation of what changes were made and why, beyond just the issue reference.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding preselection support to Search and Browse dialogs so existing selected items are shown as already selected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

ui/app/src/components/FormsEngine/controls/NodeSelector.tsx (1)

485-485: Embedded component objectIds are included in preselectedPaths.

For embedded items, item.key is the objectId (a UUID), not a file path. These entries are harmless — they won't match any path in the Browse/Search dialogs — but including them is semantically imprecise. Consider filtering them out alongside the existing .filter(Boolean):

♻️ Suggested refinement

-				preselectedPaths: value.map((item) => item.key).filter(Boolean),
+				preselectedPaths: value.filter((item) => !item.component).map((item) => item.key).filter(Boolean),

Apply the same change at both line 485 (browse) and line 508 (search).

Also applies to: 508-508

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/app/src/components/FormsEngine/controls/NodeSelector.tsx` at line 485,
preselectedPaths currently includes embedded component objectIds because it uses
value.map(item => item.key).filter(Boolean); change the filter to exclude
non-path keys (e.g., objectIds) so only real file paths remain — for example,
replace .filter(Boolean) with a predicate that checks the key looks like a path
(contains '/' or does not match UUID pattern) where preselectedPaths is set in
NodeSelector.tsx (the value.map((item) => item.key) expression used for both the
browse and search assignments around the preselectedPaths lines).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ui/app/src/components/FormsEngine/controls/ImagePicker.tsx`:
- Line 207: ImagePicker currently passes preselectedPaths: [value] which can
become [''] when value is an empty string; update the two places that set
preselectedPaths (the browse and search calls in ImagePicker) to avoid empty
strings by filtering falsy values (e.g., use [value].filter(Boolean) or
conditional logic to pass [] when value is empty) so NodeSelector never receives
['']; reference the preselectedPaths prop and the value variable in ImagePicker
and apply the same change for both the browse and search invocations.

---

Nitpick comments:
In `@ui/app/src/components/FormsEngine/controls/NodeSelector.tsx`:
- Line 485: preselectedPaths currently includes embedded component objectIds
because it uses value.map(item => item.key).filter(Boolean); change the filter
to exclude non-path keys (e.g., objectIds) so only real file paths remain — for
example, replace .filter(Boolean) with a predicate that checks the key looks
like a path (contains '/' or does not match UUID pattern) where preselectedPaths
is set in NodeSelector.tsx (the value.map((item) => item.key) expression used
for both the browse and search assignments around the preselectedPaths lines).

[coderabbitai]

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@ui/app/src/components/FormsEngine/controls/ImagePicker.tsx`:
- Line 207: The preselectedPaths array was being populated with an empty string
when no image was selected; update both dialog flows in ImagePicker (the browse
and search dialog invocations that set preselectedPaths) to use a guarded
expression like value ? [value] : [] so they won't pass [''] when value is empty
(note value is derived from defaultValue ?? ''), ensuring both the browse and
search usages of preselectedPaths follow the same pattern.