fix(settings): address kenji daily-review audit findings 1-4 - #240
Merged
Conversation
Per @kenji audit `26a221be` against #237 main `4e0247f6`: **#1 — bottom 「想先看看效果？」 hero buttons left-aligned.** PR #236 right-aligned the top 「打开每日回顾」 button via `.settingsFeatureStatusHeroActions` but the bottom 「生成每日回顾 / 生成深度分析」 row still used inline `style={{ display: 'flex', gap: 8, marginTop: 8 }}`, so it stayed glued to the paragraph margin. Route the bottom row through the same class. Add `gap: 8px` to the class so multi-button clusters keep proper spacing. **#2 — 执行时间 disabled when the master switch is off.** The disable condition was `formDisabled || savingKey === 'executeTime' || !(effectiveConfig?.enabled ?? false)`. That last clause blocked the common UX of "pick a time first, then turn it on". Drop it — the time value is harmless to edit while disabled (it doesn't fire until the switch flips), and the disabled state was misread as "broken UI". **#3 — 分析模型 default-row label still verbose.** WAWQAQ's directive: "所有模型选择只用模型名". PR #236 went from `使用对话默认模型（Codex OAuth · email · gpt-5.5）` to `对话默认（gpt-5.5）` — better, but the model id parenthetical still mixes "selectable model" with "follow chat default". Per @kenji's "don't conflate the two semantics", the default row becomes plain `跟随对话默认`; the explicit options carry the model id. **#4 — duplicate model ids across connections looked unselectable.** When two enabled connections both expose `gpt-5.5`, the flat `model`-only labels read as two identical entries. Detect the collision at build time and append `· ${connection.name}` ONLY to the colliding entries; unique model ids stay terse per WAWQAQ's directive. Deferred (with @kenji concurrence): - **#5** (`data-control-width` semantic API replacing CSS-selector width patches) — pure refactor, no end-result change. WAWQAQ explicitly said 「只看最后的结果」. - **#6** (collapse Daily Review hero card pile into a single grouped settings surface) — bigger visual restructure judgment call; needs WAWQAQ sign-off before swinging at the whole page layout.
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
1 participant
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.
Summary
Acts on @kenji's daily-review audit (thread msg `26a221be`) against #237 main `4e0247f6`. Verified each finding against current source before fixing.
Fixed:
.settingsFeatureStatusHeroActionsflex+right-align class as the top button; addedgap: 8pxto support multi-button clusters.!(effectiveConfig?.enabled ?? false)from the disable condition. Pick a time first, then enable — common flow no longer blocked.对话默认（gpt-5.5）→跟随对话默认. Per WAWQAQ "只用模型名": the default-row doesn't pick a model, it follows the conversation default; model id only appears on explicit-pick rows.· ${connection.name}disambiguation; unique model ids stay terse.Deferred (with kenji concurrence):
data-control-widthsemantic API to replace CSS selector width patches — pure refactor, no end-result change.Test plan