fix(combobox): show selected values in multi-select trigger label#4721
Conversation
The collapsed trigger was reading only `selectedOption` (the single-value path) and falling back to the placeholder when nothing matched, so a multi-select dropdown with 1+ checked items still rendered "Select one or more channels" instead of the actual selections. Added `multiSelectLabel` derived from `multiSelectValues`: - 1 value → that label - 2 values → "A, B" - 3+ → "A, B +N" Trigger now prefers `multiSelectLabel` when present and falls back to the single-select label / placeholder otherwise. Muted-text color also flips off when multi has any selection.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Updates the add/edit connector modals to show config-field descriptions via an Reviewed by Cursor Bugbot for commit 37a34cf. Configure here. |
Greptile SummaryThis PR fixes the multi-select Combobox trigger always showing the placeholder by computing a
Confidence Score: 5/5The change is a targeted UI fix with no effect on data persistence, API calls, or business logic — safe to merge. All four files make narrow, well-scoped changes: the combobox label logic is straightforward and correctly handles the empty, single, double, and overflow cases; the modal tooltip refactor is purely presentational; and the test improvements make previously flaky tests reliable without altering their assertions. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Combobox trigger renders] --> B{multiSelect mode?}
B -- No --> C{selectedOption?}
C -- Yes --> D[Show selectedOption.label]
C -- No --> E[Show placeholder\nmuted style]
B -- Yes --> F{multiSelectValues\nnon-empty?}
F -- No --> C
F -- Yes --> G{Count}
G -- 1 --> H[Show label]
G -- 2 --> I["Show 'A, B'"]
G -- 3+ --> J["Show 'A, B +N'"]
Reviews (2): Last reviewed commit: "improvement(kb-connectors): restore fiel..." | Re-trigger Greptile |
Removed 41 inline `description:` lines from configFields across 16 connectors (Slack, MS Teams, GCal, Gmail, Notion, Linear-adjacent, Discord, Dropbox, Evernote, Fireflies, Google Sheets, Intercom, Obsidian, Outlook, Reddit, ServiceNow, WordPress, Zendesk). They mostly restated the field title (e.g. "Channels to sync messages from" under a "Channels" label) and cluttered the add/edit modal. Field titles + placeholders already communicate intent. Connector-level `description` (used in the connector picker grid) is unchanged.
… polling
The "follower does a final read after timeout" test (and the
"follower returns null after timeout" test) relied on real-clock
`setTimeout` and `Date.now()` with very tight bounds (pollIntervalMs=5,
maxWaitMs=9). Any CI scheduler jitter of >4ms would cause the second
in-loop poll to be skipped, the polls counter to end at 2 instead of 3,
and the assertion `expect(result).toBe('late-leader')` to fail.
Switched both tests to `vi.useFakeTimers()` so the schedule is driven by
mocked time advanced via `vi.advanceTimersByTimeAsync`. The intent is
unchanged — verify that the in-loop deadline triggers exactly one
post-deadline last-chance call to `onFollower` — but the assertions no
longer depend on wall-clock timing.
Verified across 5 sequential runs with zero flakes.
…ooltips Restores the 41 field-level `description` lines stripped in fc64421, but instead of rendering them as inline muted-text paragraphs they're shown via a small Info icon next to each field title. Hovering or focusing the icon reveals the description in the existing emcn Tooltip. Keeps the modal layout tight while preserving the per-field guidance. Used <button type="button"> as the tooltip trigger (Radix asChild) rather than <span tabIndex={0}> to satisfy a11y/noNoninteractiveTabindex.
|
@greptile |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 37a34cf. Configure here.
1 participant
Summary
selectedOptionand ignoredmultiSelectValuesmultiSelectLabelso the collapsed trigger shows the selection: 1 → label, 2 → "A, B", 3+ → "A, B +N"Type of Change
Testing
Tested manually — the trigger now reflects the selected channels in the dropdown.
Checklist