Repository navigation
fix(selectors): preserve controls through loading transitions - #2156
Merged
Merged
Conversation
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
Switching between conversations that both have no selected model briefly replaced “Select model” with “Loading” and rebuilt the trigger. Shared selectors also replaced their button DOM when tooltip availability changed. Selected-model controls changed structure while disabled, removing the effort control, and the mobile picker used CSS-only disabling while changing editable presentation during transient loading.
Solution
Keep loading as an interaction gate while preserving the empty action label. Retain the shared SelectorPill tooltip owner and centralize accessible disabled explanations there, forwarding them through PillGroup. Preserve combined and separate model/effort controls while disabled; immediately hide and close their menu, and reset its transient preview when reopened. Mobile uses the same native disabled behavior without treating a temporary disabled state as a change in editing capability.
Conversation bindings and mobile configuration remain the source of truth. This adds no model cache, persistence changes, requests, or previous-conversation fallback. The branch selector already retains known names and is unchanged. This PR contains only selector continuity, its regressions, and UI/lifecycle audit reports.
Potential risks
SelectorPill is shared, so tooltip ownership and the optional disabled explanation wrapper affect workspace and model callers. Callers configuring disabledTooltip must retain that prop across enabled/disabled states to keep the focus host stable. A disabled model menu is intentionally dismissed. Tooltip owners now remain mounted even without content; native idle memory/frame-time measurements have not been collected.
Native desktop switching and mobile device visual verification remain unverified. Two existing browser layout tests were skipped because ORGII_HEADLESS_CHROME was not configured. No database, dependency, IPC, or persisted-format changes are included; rollback is a normal revert of this UI-only commit.
Verification
Rebased onto the latest fetched develop in an isolated worktree, carrying only this task’s changes:
pnpm test src/components/ModelSelectorPill/ModelSelectorPill.test.ts src/components/SelectorPill src/components/PillGroup/index.test.ts src/modules/MobileRemote/components/composer/MobileModelPicker.test.ts src/engines/ChatPanel/InputArea/components/ModelPill.test.ts src/engines/ChatPanel/InputArea/components/ModelPill.memberOwnership.test.ts src/features/SessionCreator/components/__tests__/SessionInfoPillGroup.test.ts src/features/SessionCreator/components/__tests__/SessionInfoLine.test.ts src/features/SessionCreator/components/__tests__/buildSessionInfoSegments.test.ts— 53 passed, 2 existing browser layout tests skipped, 10 files passed.pnpm typecheck:fast— passed.npx lint-staged— oxlint, ESLint and Prettier passed; staged TypeScript gate also passed.git diff --check origin/develop...HEAD— passed.Visual evidence
No new native screenshots or switching recording are attached: the ORG2 Dev window is unavailable on the current automation surfaces. Static images would not establish whether the transition flickers; DOM identity/interaction regressions provide partial evidence, but native visual verification is still outstanding. No measured performance improvement is claimed.
Audit
docs/frontend-ui-audit-2026-09-27/SelectorContinuity.mdanddocs/org2-performance-guard-2026-09-27/SelectorContinuity.md.