Skip to content

Enhance Select for robust object value support (#3893)#4303

Merged
jskupsik merged 12 commits into
developfrom
enhance-select-object-support
Jun 26, 2026
Merged

Enhance Select for robust object value support (#3893)#4303
jskupsik merged 12 commits into
developfrom
enhance-select-object-support

Conversation

@jskupsik

@jskupsik jskupsik commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Fixes bugs preventing object (non-primitive) values from working correctly, and adds a generateOptionFn prop for resolving values not present in the current options list.

Bug fixes for object values

  • doQueryAsync: dedupe via isEqual/unionWith instead of keyBy (string-key collision on objects).
  • optionRenderer / selected-option matching: use deep equality so object values match and no longer render as [object Object] or collide with one another.

New generateOptionFn prop

  • Synchronous (value) => SelectOption called when a selected value is not found among the current options (e.g. queryFn-based selects, readonly forms). Return null to fall back to the default value-as-label behavior.
  • Resolves the label synchronously in findOption, so the value renders with its proper label on first paint - no async cache, reaction, or race conditions.

This supersedes the async lookupFn + _lookupCache approach earlier explored on this branch; the synchronous design is a much smaller, simpler footprint (just the prop plus the findOption fallback) and avoids the stale-cache / reaction-ordering bugs that approach was prone to.

Tested on desktop and mobile via the Toolbox companion PR (employee-by-id example renders the label, not the raw id, on mount, for a value not in the options list).

Hoist P/R Checklist

Pull request authors: Review and check off the below. Items that do not apply can also be
checked off to indicate they have been considered. If unclear if a step is relevant, please leave
unchecked and note in comments.

  • Caught up with develop branch as of last change.
  • Added CHANGELOG entry, or determined not required.
  • Reviewed for breaking changes, added breaking-change label + CHANGELOG if so.
  • Updated doc comments / prop-types, or determined not required.
  • Reviewed and tested on Mobile, or determined not required.
  • Created Toolbox branch / PR, or determined not required.

If your change is still a WIP, please use the "Create draft pull request" option in the split
button below to indicate it is not ready yet for a final review.

Pull request reviewers: when merging this P/R, please consider using a squash commit to
collapse multiple intermediate commits into a single commit representing the overall feature
change. This helps keep the commit log clean and easy to scan across releases. PRs containing a
single commit should be rebased when possible.

Fix bugs preventing object values from working correctly:
- valueToOption: resolve labels via labelField/valueField instead of toString()
- doQueryAsync: use isEqual for dedup instead of keyBy (string-key collision)
- optionRenderer: use deep equality for selected-check comparison

Add lookupFn prop for async resolution of values not in the current options
list, with a separate _lookupCache to avoid dropdown leakage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jskupsik

Copy link
Copy Markdown
Contributor Author

Testing lookupFn -

Before using lookupFn:

  • Mobile:
image
  • Desktop:
image

After using lookupFn:

  • Mobile:
image
  • Desktop
image

@jskupsik

Copy link
Copy Markdown
Contributor Author

Toolbox PR: xh/toolbox#829

jskupsik and others added 3 commits March 16, 2026 12:43
The object-handling branch was conceptually wrong — valueToOption wraps
an arbitrary value INTO an option, it should not try to extract
labelField/valueField from the value itself. The lookupFn prop is the
proper mechanism for resolving display labels for unknown object values.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ect-support

# Conflicts:
#	desktop/cmp/input/Select.ts
#	mobile/cmp/input/Select.ts
@jskupsik jskupsik marked this pull request as ready for review June 25, 2026 14:03
@lbwexler lbwexler self-assigned this Jun 25, 2026
@lbwexler

Copy link
Copy Markdown
Member

Could we make the lookupFunction synchronous? Is is more of a optionGenerator? This would simplify things greatly and assuage some concerns I am uncovering with timing and loops, and alot of the complexity.

@jskupsik jskupsik merged commit c4696b6 into develop Jun 26, 2026
3 checks passed
@jskupsik jskupsik deleted the enhance-select-object-support branch June 26, 2026 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants