From b6d08fb0e61c852f314157502506579c2a9f6416 Mon Sep 17 00:00:00 2001 From: Waleed Date: Fri, 22 May 2026 08:52:18 -0700 Subject: [PATCH] fix(combobox): show selected values in multi-select trigger label (#4721) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(combobox): show selected values in multi-select trigger label 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. * chore(kb-connectors): strip redundant field-level descriptions 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. * test(leader-lock): use fake timers to deterministically test follower 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. * improvement(kb-connectors): restore field descriptions as info-icon tooltips Restores the 41 field-level `description` lines stripped in fc644210d, 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 + + {field.description} + )} - + {hasCanonicalPair && canonicalId && ( @@ -372,9 +388,6 @@ export function AddConnectorModal({ )} - {field.description && ( -

{field.description}

- )} {field.type === 'selector' && field.selectorKey ? (
- +
+ + {field.description && ( + + + + + {field.description} + + )} +
{hasCanonicalPair && canonicalId && ( @@ -406,9 +422,6 @@ function SettingsTab({ )}
- {field.description && ( -

{field.description}

- )} {field.type === 'selector' && field.selectorKey ? ( { + if (!multiSelect || !multiSelectValues || multiSelectValues.length === 0) return null + const labelFor = (v: string) => allOptions.find((opt) => opt.value === v)?.label ?? v + if (multiSelectValues.length === 1) return labelFor(multiSelectValues[0]) + if (multiSelectValues.length === 2) { + return `${labelFor(multiSelectValues[0])}, ${labelFor(multiSelectValues[1])}` + } + return `${labelFor(multiSelectValues[0])}, ${labelFor(multiSelectValues[1])} +${multiSelectValues.length - 2}` + }, [multiSelect, multiSelectValues, allOptions]) + /** * Filter options based on current value or search query */ @@ -590,11 +606,11 @@ const Combobox = memo( - {selectedOption ? selectedOption.label : placeholder} + {multiSelectLabel ?? (selectedOption ? selectedOption.label : placeholder)} { it('follower does a final read after timeout to catch a just-finished leader', async () => { redisConfigMockFns.mockAcquireLock.mockResolvedValueOnce(false) - // pollInterval=5, maxWait=9 → loop exits after 2 in-loop polls (T+5, T+10); - // the third call (polls=3) is the post-deadline last-chance read. - let polls = 0 - const onFollower = vi.fn(async () => { - polls += 1 - if (polls <= 2) return null - return 'late-leader' - }) + /** + * The intent: after the in-loop poll deadline is reached, the follower + * does exactly one more (last-chance) `onFollower` call to catch a leader + * that finished between the previous poll and the timeout. Using fake + * timers makes the timing deterministic — pollInterval=10 and maxWait=15 + * cause two in-loop polls (T+10, T+20) and one last-chance read (T+20), + * but the schedule is driven by mocked time, not the CI wall clock. + */ + vi.useFakeTimers() + try { + let polls = 0 + const onFollower = vi.fn(async () => { + polls += 1 + if (polls <= 2) return null + return 'late-leader' + }) - const result = await withLeaderLock({ - key: 'k', - pollIntervalMs: 5, - maxWaitMs: 9, - onLeader: async () => 'should-not-run', - onFollower, - }) + const promise = withLeaderLock({ + key: 'k', + pollIntervalMs: 10, + maxWaitMs: 15, + onLeader: async () => 'should-not-run', + onFollower, + }) - expect(result).toBe('late-leader') - expect(onFollower).toHaveBeenCalledTimes(3) + await vi.advanceTimersByTimeAsync(30) + const result = await promise + + expect(result).toBe('late-leader') + expect(onFollower).toHaveBeenCalledTimes(3) + } finally { + vi.useRealTimers() + } }) it('follower returns null after timeout', async () => { redisConfigMockFns.mockAcquireLock.mockResolvedValueOnce(false) - const result = await withLeaderLock({ - key: 'k', - pollIntervalMs: 5, - maxWaitMs: 20, - onLeader: async () => 'should-not-run', - onFollower: async () => null, - }) + vi.useFakeTimers() + try { + const onFollower = vi.fn(async () => null) + const promise = withLeaderLock({ + key: 'k', + pollIntervalMs: 10, + maxWaitMs: 25, + onLeader: async () => 'should-not-run', + onFollower, + }) - expect(result).toBeNull() + await vi.advanceTimersByTimeAsync(50) + const result = await promise + + expect(result).toBeNull() + } finally { + vi.useRealTimers() + } }) it('only one of N concurrent callers acquires the lock', async () => {