-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(gui): skip disabled catalog actions on ArrowDown #4337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # Catalog chain readiness | ||
|
|
||
| Review the existing catalog chain without duplicating its implementation. The lane produces a precise integration handoff; an additional product PR exists only if a concrete defect or necessary regression gap remains. | ||
|
|
||
| Loop: satisfy-spec, triggered by the catalog lane delegation. Class C3 review; security changes would promote their slice to C4. Goal: readiness for #4325 -> #4328 -> #4331. Non-goals: merging, original branch writes, issue closure, release, services and configuration. Local suites of every size are NOT RUN, including wrappers; large local install/build/typecheck are also excluded. Only task-owned files and scoped commits/push --no-verify/PR creation are authorized. Existing GitHub credentials only; no user token/time/agent-count bound. | ||
|
|
||
| Verifier: live gh PR/review/run JSON and Git ancestry/source inspection observe exact catalog tips; product suites execute only on hosted CI. Text checks observe these documents, never establish product test success. Stop: durable exact-head evidence/disposition plus honest remaining acceptance. Outcomes: DONE for completed readiness scope, NOOP for existing sufficient implementation, NEEDS_HUMAN for unresolved integration decisions, BLOCKED only for demonstrated unavailable prerequisites. Memory artifact: this unit and ignored .tmp/catalog-review/HANDOFF.md. Escalation: original-task write collision or necessary authority beyond scope goes to parent; two failed distinct reviewer calls are reclaimed with independent-review gap recorded. | ||
|
|
||
| Dependency order: roadmap (010), scoped review/coverage (020), final hosted CI and GUI evidence (030). No native stacks. Preserve public author commits. Source of truth: structure/gui-and-management-api.md, changed only if a product contract changes. No new fields, enums, enforcement or interfaces planned. Native architect role is not exposed; supported inherited-model design review and reflection provide consultation per explicit user direction, without claiming native role selection. User explicitly instructs independent work to continue when tools are unavailable. | ||
|
|
||
| Parent scope correction: existing owner is merging its own chain; this lane reconciles read-only, with own follow-up only for a concrete newly verified defect. Supported inherited-model independent design review replaces the unavailable native-role transport per explicit user direction, without claiming native architect selection. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # Roadmap documentation cycle | ||
| NEW 000_plan.md and decade documents 010/020/030 in this unit; before: absent; after: outcome, authority, exact read targets and acceptance. NEW .tmp/catalog-review/HANDOFF.md: identity, current PR states and evidence pointers. No product delta. Check: read all four documents and git diff --check; confirm every phase has real outputs and user restrictions. D locks this roadmap and directs the next cycle to review exact tip source. | ||
2 changes: 2 additions & 0 deletions
2
devlog/_plan/260912_catalog_lane_readiness/011_design_reflection.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # Design review dispositions | ||
| Inherited-model read-only reviewer Hilbert supplied CAT-DEC-01..06. Main accepts evidence ownership (01), exact-head provenance (02), no-suite restrictions (05). Amended collision boundary (03) to require a concrete new defect plus owner/head refresh and parent coordination. Amended consultation (04) to distinguish supported independent design reflection from unavailable native architect role. Amended integration authority (06) to preserve separately authorized original-owner merges and parent-only follow-up integration. Reflection recheck requested after amendments. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| # Scoped review and coverage cycle | ||
| Depends on roadmap. READ exact #4331 head gui/src/components/AddProviderModal.tsx, provider-catalog/ProviderCatalog.tsx, CatalogAccountRow.tsx, ProviderNoteModal.tsx, provider-presets.ts, gui/tests/provider-catalog-search.test.tsx and tests/gui/provider-workspace-data.test.ts. READ #4328 diff and live review threads. NEW .tmp/catalog-review/020_review.md: file:line findings, dispositions, remaining acceptance and attribution. Before: no independent lane review; after: a checked result against the pinned SHA. | ||
|
|
||
| Potential MODIFY gui/tests/provider-catalog-search.test.tsx only for a concrete defect absent from the existing owner work, after refreshing owner/head evidence and parent coordination. A coverage gap alone is not authority to duplicate the owner delivery. Activate Escape with a nonempty query then empty query; expect query clear before modal close. Activate ArrowDown from search with a disabled first account control and later enabled controls; expect the first enabled result to receive focus. Also inspect no-actionable-result behavior and note-popup -> query -> dialog Escape ordering. If required, append a precise repair work-phase at P and use a separate task-owned child branch of the refreshed final tip; never edit the original branch. Reuse existing tests and source docs, no speculative abstraction. No test execution locally. Check: independent source review, diff --check and GitHub-hosted CI for any new code. If no patch is justified, record NOOP explicitly. |
13 changes: 13 additions & 0 deletions
13
devlog/_plan/260912_catalog_lane_readiness/022_keyboard_repair.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # Keyboard repair plan | ||
|
|
||
| Previous D locked the docs-only roadmap; source reconciliation now establishes one new defect. The original owner merged #4331 at 9a37813593514c2d90b1ebac129c4541fd2a9af4; its reviewed source tip a154645d76e98af199fd79aff8c8d393afaf30ab passed hosted CI 34672274572. This task rebased only its own unpushed roadmap commit onto that dev tip. Parent was notified of the new distinct defect; original task scope readback shows only tab overflow, description disclosure and popup focus repairs. | ||
|
|
||
| Class C1 behavioral patch plus existing-test coverage; no new abstraction, type, field, token, endpoint, UI copy or dependency. Do-nothing would retain a broken keyboard path; configuration cannot change the selector; reuse the existing handler and test mount. Product diff is confined to the existing selector. | ||
|
|
||
| MODIFY gui/src/components/provider-catalog/ProviderCatalog.tsx:207: before querySelector("button, a[href]"); after querySelector("button:not(:disabled), a[href]"). CSS :disabled also excludes a disabled fieldset descendant, while preserving actionable anchors. | ||
|
|
||
| MODIFY gui/tests/provider-catalog-search.test.tsx: append behavior tests using current mount/type/search helpers. Busy openai Codex row (logged out, onAccountLogin supplied), query nvidia: disabled account button is first in DOM, ArrowDown must focus NVIDIA preset and prevent default. Same busy account with unmatched query and no other actionable row: focus stays on search and default remains untouched. Empty results: same no-op. A normal preset-only query checks normal first-result focus. All tests dispatch a bubbling/cancelable KeyboardEvent from the focused input inside act. No sleep helper or exported test-only production function. | ||
|
|
||
| MODIFY structure/gui-and-management-api.md Add provider row: ArrowDown focuses first enabled result action; no available action leaves input focus unchanged. MODIFY docs-site/src/content/docs/guides/web-dashboard.md Add provider row with the same keyboard behavior, translated pages must not contradict (they currently say nothing about this shortcut). | ||
|
|
||
| Verification: git diff --check for patch formatting ONLY, independent source audit, GitHub-hosted Cross-platform CI at the exact published head. No local test/build/typecheck/install. Read hosted preview artifact from that run if GUI evidence requires it; serve artifact in scratch without product build, no live proxy mutation. No original branch/PR mutation, merge or auto-merge. Existing author commits stay in ancestry. Follow-up ordinary PR targets dev because original chain is now merged. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # Final evidence and handoff cycle | ||
| Depends on reviewed source/repair disposition. READ live gh pr view for #4325/#4328/#4331 and any follow-up, GraphQL reviewThreads, gh run view for exact head, workflow triggers and Git ancestry. READ screenshots carried by the source PR using local git blobs; observe them with image viewer, distinguish screenshot commit from final source head. NEW .tmp/catalog-review/030_evidence.md with CI run IDs/URLs, job outcomes, missing/skipped distinctions, GUI provenance and outstanding reviews. MODIFY .tmp/catalog-review/HANDOFF.md from preliminary to complete: worktree/branch, own phase/cycle evidence, all original PR dispositions, final chain/head SHAs, authors, remaining acceptance and local tests NOT RUN. No product change. Check: exact SHA equality between PR and CI plus fresh PR state; do not claim intermediate tips passed. This lane never merges or retargets. Reconcile the original-chain delivery by its separately authorized existing owner; parent controls additional follow-up integration. | ||
|
|
||
| Inspect exact-tip hosted dashboard preview artifact if available; compare narrow-width tabs and popup focus against historical screenshots. If preview cannot be exercised within authorized no-build/no-install scope, leave final-tip dynamic GUI acceptance explicitly unmet, not inferred from old PNGs. | ||
|
|
||
| Record observation timestamp, base SHA, workflow event/run attempt and merge commit. Head/base movement triggers reconciliation refresh. |
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 10618
🤖 get_repo_knowledge executed:
get_repo_knowledge lidge-jun/opencodex /tmp/coderabbit-repo-knowledge/lidge-jun-opencodex-7afea732/conventions /tmp/coderabbit-repo-knowledge/lidge-jun-opencodex-7afea732/learningsLength of output: 16028
Define the readiness document scope
devlog/_plan/260912_catalog_lane_readiness/011_design_reflection.mdis in the same unit and recordsCAT-DEC-01..06, including amended authority boundaries. Line 2 names only000_plan.md,010_roadmap.md,020_review.md, and030_evidence.md, so the required review can omit decisions that govern the acceptance check. Include011_design_reflection.mdin the read set, or state its intentional exclusion.🤖 Prompt for AI Agents