Flat design system: Phases 1-4 token layer + UI cleanup#2374
Conversation
5d8700d to
c2eb746
Compare
|
CI runs are blocked with |
There was a problem hiding this comment.
Thanks for the design system work — the centralized DesignTokens.h plus the migration of Theme.h / ComboStyle.h to use it is a nice cleanup, and the txActive dynamic property pattern in TxApplet is a clear win over the old "rebuild the whole stylesheet on every state change" approach. Removing the custom frameless title bar in NetworkDiagnosticsDialog in favor of native chrome also drops a meaningful chunk of duplicated drag/resize/event-filter code. A few things worth a look:
1. PanadapterApplet border stylesheet probably won't render. In PanadapterApplet.cpp:
setStyleSheet(
"PanadapterApplet { border: 1px solid " + DesignTokens::kBorderControl + "; "
"border-radius: 4px; }");A class-selector stylesheet on a QWidget subclass that does not override paintEvent and does not have Qt::WA_StyledBackground set is silently dropped by Qt — this is the same trap ContainerWidget already handles correctly in this PR (the new paintEvent + setAttribute(Qt::WA_StyledBackground, true) pair). PanadapterApplet has neither (PanadapterApplet.h:18 is a plain QWidget subclass with no paintEvent), so this border rule is dead. Either drop the rule, or do what ContainerWidget does (add setAttribute(Qt::WA_StyledBackground, true) and a paintEvent that calls style()->drawPrimitive(QStyle::PE_Widget, …)). The PR test plan only checks sidebar applets (which get their border from ContainerWidget), so this would have passed visual review even though the panadapter itself isn't actually getting the intended border.
2. QColor(kTextSecondary.data()) in ComboStyle.h is fragile.
p.setBrush(QColor(kTextSecondary.data()));QLatin1StringView::data() is not guaranteed to be null-terminated in general; it happens to be here because the view is constructed from a string literal, but it's a footgun if anyone later changes the construction site. Safer alternatives that don't depend on null-termination: QColor(QLatin1String(kTextSecondary)) or QColor::fromString(kTextSecondary).
3. QColor::setNamedColor() is deprecated since Qt 6.6. In ContainerWidget::paintEvent:
QColor bg;
bg.setNamedColor(DesignTokens::kSurfacePanel);Prefer QColor bg = QColor::fromString(DesignTokens::kSurfacePanel); (same for border). Functional today but will spit deprecation warnings on Qt ≥ 6.6.
4. Width bump 260 → 280 is a user-visible change. AppletPanel::kWidth = 280 is called out in the PR body, but worth confirming that the +20px on the right column is intended for everyone — users with saved/expected layouts will see the center pane shrink by 20px on next launch (the existing s.remove("SplitterState") already wipes saved state, so the new sizing applies immediately).
Nothing here is a blocker — #1 is the one I'd want addressed (or the dead rule deleted) before merge. Nice work on the token layer.
Introduce DesignTokens.h vocabulary, rewrite Theme.h to use tokens, and consolidate stylesheet overrides across 15 files. Includes card borders via QPainter, gradient→flat title bars, spacing/layout fixes (kWidth 280, 6px stack gap), and NetworkDiagnosticsDialog native title bar conversion. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
c2eb746 to
6bea0df
Compare
|
Rebased onto latest
Builds clean at 6bea0df. |
…amedColor deprecation - Remove PanadapterApplet border stylesheet — class-selector rule on QWidget without paintEvent or WA_StyledBackground is silently dropped by Qt - Replace QLatin1StringView::data() with QColor::fromString() in ComboStyle.h - Replace deprecated setNamedColor() with QColor::fromString() in ContainerWidget
6bea0df to
f3b22e3
Compare
|
Claude here — first, thanks for the substantial work, MARTIN. The token vocabulary you proposed (kSurfaceBase, kBorderControl, kColorAccent, etc.) is sound and maps cleanly onto the W3C Design Tokens Format that's the modern industry standard. After working through this with Jeremy, the project direction is going to be slightly different from what this PR proposes:
So this PR as it stands would need to be reworked rather substantially:
That's effectively a different PR. Two paths forward: A. Close this PR; we open #2453 fresh for the foundation work, you (or anyone) can take a stab at it with the new spec. B. Keep this PR open while you rework it against #2453's spec — the token vocabulary you've already established here directly informs the JSON structure. Either is fine — your call. The visual flat design ideas can also become the There's also a real conflict with #2449 (just merged) on NetworkDiagnosticsDialog that would need rebasing regardless — that PR added per-dialog frameless support that this PR's NetworkDiagnostics rewrite would silently regress. Thanks for the work and patience. 73, Jeremy KK7GWY & Claude (AI dev partner) |
|
Closing this one as per the recommended option A. |
Summary
Implements RFC #2294 — Flat Design System, Phases 1-4:
DesignTokens.hvocabulary, Theme.h rewritten to use tokens, ComboStyle.h token substitutionTest plan
cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo && cmake --build build -j$(nproc)🤖 Generated with Claude Code