Adjust Card Chooser search bar to include Realm picker#3941
Adjust Card Chooser search bar to include Realm picker#3941
Conversation
Preview deployments |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f8d335944
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @onChange={{this.onChange}} | ||
| @placeholder={{@placeholder}} | ||
| @disabled={{@disabled}} | ||
| @renderInPlace={{this.renderInPlace}} | ||
| @matchTriggerWidth={{true}} | ||
| @renderInPlace={{@renderInPlace}} | ||
| @matchTriggerWidth={{@matchTriggerWidth}} |
There was a problem hiding this comment.
By switching to @renderInPlace={{@renderInPlace}} without a default, any Picker usage that doesn’t pass @renderInPlace now sends undefined into Ember Power Select. ember-basic-dropdown treats undefined as false, so the dropdown renders to the document body instead of in-place. This is a behavior change from the prior default (always in-place) and will cause pickers like the new realm picker to escape overflow/positioning contexts (e.g., inside the search sheet) and can break click-outside behavior and styling. Consider preserving the previous default (true) when the argument is omitted.
Useful? React with 👍 / 👎.
backspace
left a comment
backspace
left a comment
There was a problem hiding this comment.
I tried this via the preview deployment, looks good. Maybe it’s called for in the design but I do find it a bit awkward that the checked realms disappear immediately upon click:
I don’t know where to find discussion of this but I think the best practice is to keep things stable until the cursor has left the area, because if I changed my mind about including a realm, I now have to scroll up to find it to unclick it
|
|
||
| get renderInPlace() { | ||
| return this.args.renderInPlace ?? true; | ||
| } |
There was a problem hiding this comment.
I don’t know all the places this is used so maybe Codex’s comment is overly imaginative, but it does seem sensible to me to provide a default for this vs leaving it undefined if the consumer doesn}t pass anything in
| type: 'option', | ||
| }); | ||
| } | ||
| console.log(options); |
There was a problem hiding this comment.
Did you mean to leave this in?
| } | ||
| .realm-picker__loading-label { | ||
| font-weight: 500; | ||
| } |
There was a problem hiding this comment.
These are probably an LLMism but IMO since we’re using scoped CSS there‘s no reason to use BEM-style class names.
2 participants
Screen.Recording.2026-02-04.at.15.28.47.mov