feat(gui): make the add-provider search reach every tab at once - #4331
Conversation
|
✅ Deterministic PR hygiene checks passed. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
리뷰 · 우선순위 63 / 80지금 이 PR(#4331)은 그 불편을 고칩니다. 검색창을 탭 위로 올리고, 글자를 치는 동안에는 탭을 바꾸지 않은 채 Accounts·Free·Local·Paid 네 그룹을 한 화면에 묶어서 보여 줍니다. 탭 줄은 잠깐 검색 문자열은 다만 이 브랜치 베이스는
문서( 메인테이너의 판단이 필요한 지점
너의 추천
이 댓글은 grok-bot이 작성했습니다 |
The search box sat under the tab strip and filtered only the selected tab, and a tab click wiped whatever had been typed. Someone looking for Claude on the Free tab saw nothing and had no way to find out it was one tab over. The input now sits above the tabs and matches across all four. Search mode replaces browse mode rather than filtering inside it. While a query is live every group renders - Accounts, Free, Local, Paid - each under a small-caps heading with its count, and the selected tab is frozen rather than moved. Moving it would not merely change which rows are listed: it changes the kind of row, because a preset-select button becomes a login row with Log in and Add account buttons. The strip drops role=tablist for that duration and becomes jump chips with counts, disabled at zero, that scroll to a group without changing the tab. Clearing the query restores the tab the user chose and returns the list to the top. Account login rows are searchable too, and the row with a login in flight is pinned into the results regardless of the query: it owns the authorization URL and the paste field, and unmounting it mid-login throws away what the user is doing. A preset sharing an id with a matched login row is dropped so the same provider cannot appear twice. The haystack stays label and id. Matching adapters would return Ollama, vLLM, LM Studio, Groq, Cerebras and PackyCode for the query openai, and matching base URLs would return every local row for localhost. It widens in exactly two controlled ways instead: an equality match on the adapter id, and a small alias set resolved through the local classifier. Ranking is per group only - exact, then prefix, then the caller order that already carries the sponsor pin and usage rank - because a paid sponsor sorted above free NVIDIA on the query nim reads as an ad slot. The search text is owned by AddProviderModal because its Escape listener is on window and registers before a child's, so Escape unwinds one layer at a time: the note popup, then a live query, then the dialog.
414ec2d to
e3fdf8f
Compare
f39b5ad to
68e6b02
Compare
…vidence Four dependent PRs with their exact heads and live CI run ids, plus the two traps that make a green head look red: a cancelled duplicate run leaves a FAILURE ci row attached to the same SHA as the live green run, and a cancelled required check is not a passing one - all three enforce-target attempts on #4328 were cancelled by concurrency, which left the PR UNSTABLE until one was re-run.
…catalog-unified-search # Conflicts: # gui/src/components/provider-catalog/provider-presets.ts # structure/gui-and-management-api.md # tests/gui/provider-workspace-data.test.ts
…catalog-unified-search # Conflicts: # gui/src/components/provider-catalog/provider-presets.ts
1 participant
Summary
The search box sat under the tab strip and filtered only the selected tab, and a tab click wiped whatever had been typed. Someone looking for Claude while the Free tab was open saw nothing, with no hint that it was one tab over. The input now sits above the tabs and reaches all four at once.
Search mode replaces browse mode rather than filtering inside it. While a query is live every group renders - Accounts, Free, Local, Paid - each under a small-caps heading with its count, and the selected tab is frozen rather than moved.
Moving it would not merely change which rows are listed: it changes the kind of row, because a preset-select button becomes a login row with Log in and Add account buttons. That is the VS Code Extensions-view failure (Marketplace vs Installed vs Updates), and it is why there is no auto-switch here. Scroll position could not be restored honestly across such a jump either, because the destination is a different dataset.
The strip drops
role="tablist"for the duration and becomes jump chips with counts, disabled at zero, that scroll to a group without changing the tab. Keepingaria-selectedon a tab whose rows are no longer on screen would have a screen reader announce "Free, selected" over a Paid row. Clearing the query restores the tab the user chose and returns the list to the top.Account login rows are searchable, and the row with a login in flight is pinned into the results regardless of the query: it owns the authorization URL and the paste field, so unmounting it mid-login throws away what the user is in the middle of doing. A preset sharing an id with a matched login row is dropped so the same provider cannot appear twice.
The haystack stays label + id. Matching adapters would return Ollama, vLLM, LM Studio, Groq, Cerebras and PackyCode for the query
openai, and matching base URLs would return every local row forlocalhost. It widens in exactly two controlled ways instead: an equality match on the adapter id, and a small alias set (localhost,lmstudio,self-hosted, ...) resolved through the local classifier rather than by substring.Ranking is per group only - exact, then prefix, then the caller order that already carries the sponsor pin and usage rank. A paid sponsor sorted above free NVIDIA on the query
nimreads as an ad slot.The search text is owned by
AddProviderModalbecause its Escape listener is onwindowand registers before a child's, so Escape unwinds one layer at a time: note popup, then a live query, then the dialog.Two things worth a reviewer's eye. The chip looks its group up by a
data-catalog-groupattribute rather than by id, because group ids come fromuseId(which emits colons) and selecting on one needsCSS.escape- which does not exist in the happy-dom environment the GUI tests run in, so a chip click would have thrownReferenceErroron CI. And the jump setsscrollTopdirectly and focuses withpreventScroll, because.modal-cardis also a scroll container and either default would drag the search field out of view.This is a gui change. Grouped results across two tabs, a match that lives on a different tab from the selected one, and browse mode unchanged after clearing:
Verification
bun x tsc --noEmit- passed.cd gui && bun x tsc --noEmit -p tsconfig.json- passed (includesgui/tests).bun run structure:check- passed.localhostresolves to Local only; a chip click scrolls to its group, keeps the query, and does not drag the card; Escape closes the note popup, then clears the query, then closes the dialog.matchesCatalogQuery,sortCatalogMatches,filterAccountRowsanddropPresetsCoveredByAccountsintests/gui/provider-workspace-data.test.ts, plusgui/tests/provider-catalog-search.test.tsxfor the frozen tab, the query surviving a strip click, and the pinned in-flight login.structure/gui-and-management-api.mdand theAdd providerrow in fivedocs-sitelocales now describe the four tabs, the unified search and the note clamp.Based on #4328. Retarget to
devonce that lands.Checklist
Integration review
The owner requested merging this chain into dev. Integrating as lidge-jun under MAINTAINERS.md after exact-head remote CI passes; no self-approval is claimed. Local tests remain NOT RUN.
Contains the two corrected parent layers, merged without rewriting their published commits. Production GUI build passed; root source contract remains unchanged.
Verification target for maintainer integration:
a154645d76e98af199fd79aff8c8d393afaf30ab. Cross-platform CI: https://github.com/lidge-jun/opencodex/actions/runs/34672274572. Merge is gated on that exact-head run succeeding. Review repairs were verified with a production GUI build and browser checks; the local suite remains NOT RUN.