ref: compositeSelect gets a required trigger #103713
Merged
+47
−30
Conversation
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
github-actions
bot
added
the
Scope: Frontend
we no longer do trigger label updating
TkDodo
commented
Nov 20, 2025
Comment on lines
-174
to
-176
| // Region 1's callback is called, and trigger label is updated | ||
| // Region 1's callback is called | ||
| expect(region1Mock).toHaveBeenCalledWith({value: 'choice_one', label: 'Choice One'}); | ||
| expect(screen.getByRole('button', {name: 'Choice One'})).toBeInTheDocument(); |
Collaborator
Author
There was a problem hiding this comment.
note: we no longer automatically update trigger labels for CompositeSelect, as we always have to pass in a custom trigger.
natemoo-re
approved these changes
Nov 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
almost all CompositeSelect usages pass either a trigger or triggerProps already, which means they never need the default trigger label calculation that is done internally.
In an upcoming perf improvement, this calculation will be changed / removed because it relies on all options being rendered eagerly. This computation can be done in the parent for
CompactSelectbecause all items are available, butCompositeSelectdoes this with regions so it would be harder. However, we don’t need it if its unused.This PR just enforces that there won’t be a future usage that uses the default trigger label in CompositeSelect by always enforcing a trigger to be passed. It also unifies
triggerLabel&triggerusages, astriggerLabelis no longer allowed.