Skip to content

fix(settings): drop daily-review hero card pile + semantic width attr (kenji #5+#6) - #244

Merged
jackwener merged 1 commit into
mainfrom
yuejing/daily-review-bundled-refactor
Jun 25, 2026
Merged

fix(settings): drop daily-review hero card pile + semantic width attr (kenji #5+#6)#244
jackwener merged 1 commit into
mainfrom
yuejing/daily-review-bundled-refactor

Conversation

@jackwener

Copy link
Copy Markdown
Member

Summary

Bundled response to @kenji audit findings #5 + #6 (msg `26a221be`) plus the structural cleanup WAWQAQ authorized in msg `afbe542d` ("一次性多改,别他妈分这么多轮").

#6 — Daily Review hero card pile collapsed.
Removed the intro hero card ("每日回顾 · 本地 + LLM" with paragraph + 「打开每日回顾」 button) and the bottom "想先看看效果?" hero card (paragraph + 2 generate buttons). The page now reads as:

  • thin banner (1-line status)
  • grouped settings rows
  • single right-aligned .settingsPageFooterActions toolbar with all three actions (打开每日回顾 · 生成每日回顾 · 生成深度分析)

The .settingsFeatureStatusHero* CSS rules survive because the voice settings page still uses them.

#5 — semantic width attr.

  • .settingsRow > .settingsTimeInput.settingsRow[data-control-width=\"compact\"] > input, .settingsRow[data-control-width=\"compact\"] > .settingsTimeInput
  • .settingsRow > .settingsBaseSelectTrigger.settingsRow[data-control-width=\"select\"] > .settingsBaseSelectTrigger
  • 执行时间 row gets data-control-width=\"compact\", 分析模型 row gets data-control-width=\"select\". The contract is now declared on the row, not implicit in the control's class name.

Verified the 4 other SettingsSelect call sites (proxy host, gateway host, bot domain, usage filter) are inside .settingsField, not .settingsRow, so dropping the old global rule does not affect them.

Test plan

  • `tsc --noEmit -p apps/desktop/tsconfig.renderer.json` — clean on touched lines.
  • Settings → 每日回顾 renders: banner → 9 rows → footer toolbar (right-aligned 3 buttons).
  • 执行时间 input is ≤140px and right-aligned; 分析模型 trigger is ≤320px and right-aligned. Both come from data-control-width on their row, not from class selectors.
  • 「打开每日回顾」 / 「生成每日回顾」 / 「生成深度分析」 all fire from the footer.
  • No regression on Settings → 网络 (proxy), 高级 (gateway), 机器人对话 (bot domain), 使用统计 (usage filter) — those SettingsSelects sit inside .settingsField, not .settingsRow, and use the .settingsField[data-orient=\"horizontal\"] width contract instead.

Bundled response to @kenji audit findings #5 + #6 (msg `26a221be`)
plus the page-restructure WAWQAQ explicitly authorized
(thread msg `afbe542d`: 「一次性多改,别他妈分这么多轮」).

**Finding #6 — Daily Review settings page restructure.**
Removed the two `.settingsFeatureStatusHero` cards (intro + 「想先看看
效果?」). They were promo chrome on a workbench settings surface and
duplicated information the section banner already carries. The
「打开每日回顾」 / 「生成每日回顾」 / 「生成深度分析」 actions are now
folded into a single right-aligned `.settingsPageFooterActions`
toolbar rendered below the rows. Net: one banner, one grouped rows
surface, one action footer — no card pile.

The `.settingsFeatureStatusHero*` CSS rules stay because the voice
settings page still uses them; this PR just drops Daily Review's
dependency on them.

**Finding #5 — semantic width attr instead of class-selector patches.**
`.settingsRow > .settingsTimeInput` and
`.settingsRow > .settingsBaseSelectTrigger` were globally-scoped
selectors that implicitly assumed the row contained one specific
control type. Move the contract to a `data-control-width="compact|
select"` attribute on the row itself; mark the 执行时间 and 分析模型
rows accordingly. The old global rules are gone — only the data-attr
rules survive.

Verified the 4 other `SettingsSelect` call sites (proxy host, gateway
host, bot domain, usage filter) are inside `.settingsField`, not
`.settingsRow`, so they aren't affected by the dropped rule.
@jackwener
jackwener merged commit 51246a7 into main Jun 25, 2026
@jackwener
jackwener deleted the yuejing/daily-review-bundled-refactor branch June 25, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant