From 509ae968545008668fdeb0e2a46e012ba74d7cbf Mon Sep 17 00:00:00 2001 From: Nam Pham Dinh Thanh Date: Tue, 18 Aug 2026 14:55:42 +0900 Subject: [PATCH] Two macOS reports: both were Qt deciding, so both differed by platform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings sat closer to its icon than Dashboard and Giám sát do. Those are tree rows, laid out by the style; Settings was a QPushButton, whose icon-to-label gap is also the style's — and the two do not agree. The Windows fix for this was a 19px icon box that nudged the label 1px, which is exactly the kind of tuning that only holds on the machine it was measured on: on macOS the gap is tighter again. The row now lays itself out, 4px in and 6px between, the same two numbers the tree uses. Icon x=4 and label x=26 against the rows' 4 and 26 — equal, not close. The language drop-list showed a tick over its own text. macOS marks the current row with a checkmark and Windows does not, and the popup inherits the combo's width — which for "EN/JP/VN" is about 50px, all of it needed by the letters. widen_popup() measures the longest item plus the platform's indicator and sets the view's minimum, so the tick has its own room wherever it is drawn. That check found two more: the project picker (191px of text in a 138px popup) and the provider list (221px in 138px) were both truncating names here, tick or no tick. Neither could be reproduced on this machine, so the checks assert the property that made the bug possible — spacing we control rather than the style's, and a popup measured against text + indicator — not the platform. 21/21 checkers pass. check_nav segfaults in Qt teardown after printing its verdict, twice in one suite run and 0 times in 8 standalone runs; pre-existing. Co-Authored-By: Claude Opus 5 (1M context) --- app.py | 34 ++++++++++++++-- theme.py | 8 ++-- tools/check_combo_popup.py | 79 ++++++++++++++++++++++++++++++++++++++ tools/check_rail_align.py | 6 ++- ui/widgets.py | 19 +++++++++ 5 files changed, 136 insertions(+), 10 deletions(-) create mode 100644 tools/check_combo_popup.py diff --git a/app.py b/app.py index aa53c26..593a239 100644 --- a/app.py +++ b/app.py @@ -20,6 +20,7 @@ from . import APP_NAME, DISPLAY_NAME, __version__ from .config import PROVIDER_LABELS, AppConfig from .i18n import LANGUAGE_SHORT, LANGUAGES, get_language, on_language_changed, set_language, tr from .state import AppContext +from .ui.widgets import widen_popup from .theme import current_palette, set_active_theme, stylesheet from .core.task_scheduler import TaskScheduler from .ui.cowork_tab import CoworkTab @@ -46,6 +47,11 @@ _NAV_COLLAPSED_WIDTH = 54 # of a 1440 screen and more than a quarter of a 1280 one, where it left the # seven Kanban lanes 920px of the 1067 they need. A share behaves the same on # every monitor. +# Where a rail row starts, and how much air sits between its icon and its +# label. The tree rows get these from the style; anything laid out by hand +# beside them has to use the same two numbers or it will not line up. +_NAV_ROW_INSET = 4 +_NAV_ROW_GAP = 6 _NAV_MIN_WIDTH = 132 _NAV_MAX_SHARE = 0.22 _NAV_MAX_CEILING = 360 @@ -255,6 +261,7 @@ class MainWindow(QMainWindow): self.nav_project.setObjectName("navProjectPick") self.nav_project.setToolTip(tr("app.nav.project_pick")) self.nav_project.currentIndexChanged.connect(self._on_rail_project_pick) + widen_popup(self.nav_project) self.nav_new_chat = QPushButton(tr("cowork.new_chat")) self.nav_new_chat.setObjectName("navNewChatBtn") self.nav_new_chat.setIcon(_icon("plus")) @@ -323,12 +330,28 @@ class MainWindow(QMainWindow): # way of the ones you live in. A hairline (styled via #navrailBottom in # theme.py) separates the two lists. nvl.addWidget(self.nav_bottom, 0) - self._nav_settings_btn = QPushButton(tr("app.settings")) + # Settings reads as one more row under Dashboard / Giám sát, so its icon + # and label must start exactly where theirs do. Letting QPushButton place + # them does not achieve that: the gap it leaves between icon and text is + # the platform style's, and on macOS it is visibly tighter than the tree + # rows above — a Windows-tuned nudge only moved the mismatch. So the row + # is laid out here, in the same two numbers the tree uses: 4px in, 6px + # between. + self._nav_settings_btn = QPushButton() self._nav_settings_btn.setObjectName("navSettingsBtn") - self._nav_settings_btn.setIcon(_icon("settings")) self._nav_settings_btn.setFlat(True) self._nav_settings_btn.setCursor(Qt.PointingHandCursor) self._nav_settings_btn.clicked.connect(self._open_settings) + srow = QHBoxLayout(self._nav_settings_btn) + srow.setContentsMargins(_NAV_ROW_INSET, 6, 8, 6) + srow.setSpacing(_NAV_ROW_GAP) + self._nav_settings_icon = QLabel() + self._nav_settings_icon.setPixmap(_icon("settings").pixmap(16, 16)) + self._nav_settings_icon.setFixedSize(16, 16) + self._nav_settings_text = QLabel(tr("app.settings")) + srow.addWidget(self._nav_settings_icon) + srow.addWidget(self._nav_settings_text) + srow.addStretch(1) nvl.addWidget(self._nav_settings_btn) self._account_row = self._build_account_row() nvl.addWidget(self._account_row) @@ -652,6 +675,7 @@ class MainWindow(QMainWindow): if idx >= 0: self.nav_project.setCurrentIndex(idx) has = bool(choices) + widen_popup(self.nav_project) self.nav_project.setEnabled(has) self.nav_project_btn.setEnabled(has) self.nav_project_btn.setToolTip( @@ -742,8 +766,8 @@ class MainWindow(QMainWindow): (collapsed = icon only, label moves to the tooltip).""" # force: collapsing leaves the row spec identical, only the text changes. self._rebuild_nav(force=True) - self._nav_settings_btn.setText( - "" if self._nav_collapsed else tr("app.settings")) + self._nav_settings_text.setText(tr("app.settings")) + self._nav_settings_text.setVisible(not self._nav_collapsed) self._nav_settings_btn.setToolTip(tr("app.settings")) # Collapsed to 54px there is no room for either control's label; the # picker would be a stub of a name, so it steps aside entirely and the @@ -967,6 +991,7 @@ class MainWindow(QMainWindow): idx = self.language_combo.findData(get_language()) if idx >= 0: self.language_combo.setCurrentIndex(idx) + widen_popup(self.language_combo) self.language_combo.currentIndexChanged.connect(self._on_language_changed) who.addWidget(self.language_combo) self.theme_btn = self._build_theme_button() @@ -980,6 +1005,7 @@ class MainWindow(QMainWindow): self.provider_combo.setToolTip(tr("app.provider")) for key, label in PROVIDER_LABELS.items(): self.provider_combo.addItem(label, key) + widen_popup(self.provider_combo) idx = self.provider_combo.findData(self.ctx.config.active_provider) if idx >= 0: self.provider_combo.setCurrentIndex(idx) diff --git a/theme.py b/theme.py index 473e521..8913459 100644 --- a/theme.py +++ b/theme.py @@ -498,11 +498,9 @@ QToolButton#navProjectPickMini::menu-indicator { image: none; width: 0; } QPushButton#navSettingsBtn { background: transparent; border: none; color: $text_muted; - padding: 6px 8px 6px 4px; text-align: left; border-radius: ${radius}px; - /* A button leaves less air between icon and label than a tree row - does, so the word sat 3px left of Dashboard's. Widening the icon - box (the glyph stays 16px, left-aligned) closes the gap. */ - qproperty-iconSize: 19px 16px; + /* Padding stays at 0: the row lays its own icon and label out, so that + the spacing does not change with the platform's button style. */ + padding: 0; text-align: left; border-radius: ${radius}px; /* No side margin: Settings reads as one more row under Dashboard/Giám sát, so its icon has to start on their x. A 6px margin put it at 14 — near enough the middle of the collapsed 54px rail to look centred. */ diff --git a/tools/check_combo_popup.py b/tools/check_combo_popup.py new file mode 100644 index 0000000..5a61cd3 --- /dev/null +++ b/tools/check_combo_popup.py @@ -0,0 +1,79 @@ +"""A drop-list must have room for its text and for the tick beside it. + +macOS marks the current row with a checkmark; Windows does not. The language +combo is only as wide as "VN", and the popup inherits that width, so on macOS +the tick landed on top of the two letters. Reported from a Mac, so this checks +the property that made it possible rather than the platform. +""" +from __future__ import annotations + +import os +import sys + +sys.stdout.reconfigure(encoding="utf-8", errors="replace") + +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO.parent)) +sys.path.insert(0, str(Path(__file__).resolve().parent)) +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +from capture_screens import ( # noqa: E402 + _apply_theme, _freeze_schedulers, _isolate_home, _load_fonts) + + +def main() -> int: + sandbox = _isolate_home() + from PySide6.QtWidgets import QApplication, QStyle + + app = QApplication([]) + _load_fonts() + _freeze_schedulers() + _apply_theme(app) + + from cowork_local.config import CONFIG_DIR + assert str(sandbox) in str(CONFIG_DIR), f"isolation failed: {CONFIG_DIR}" + + from seed_demo_data import seed + seed() + + from cowork_local.app import MainWindow + from cowork_local.config import AppConfig + from cowork_local.i18n import set_language + from cowork_local.state import AppContext + + set_language("vi") + win = MainWindow(AppContext(AppConfig.load()), user_name="local") + win.resize(1400, 900) + win.show() + app.processEvents() + + fails = [] + for name in ("language_combo", "nav_project", "provider_combo"): + combo = getattr(win, name, None) + if combo is None or not combo.count(): + continue + view = combo.view() + fm = view.fontMetrics() + longest = max(fm.horizontalAdvance(combo.itemText(i)) + for i in range(combo.count())) + tick = combo.style().pixelMetric(QStyle.PM_IndicatorWidth, None, combo) + need = longest + tick + have = max(view.minimumWidth(), combo.width()) + print(f"{name:15}: chu dai nhat {longest:>4}px + dau tick {tick:>3}px " + f"= can {need:>4}px | popup rong {have:>4}px") + if have < need: + fails.append(f"{name}: popup {have}px, khong du {need}px cho chu + dau tick") + + print() + for f in fails: + print("FAIL " + f) + print("PASS moi drop-list du cho chu va dau tick" if not fails + else f"{len(fails)} problem(s)") + sys.stdout.flush() + os._exit(1 if fails else 0) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/check_rail_align.py b/tools/check_rail_align.py index ca19f13..aaee47f 100644 --- a/tools/check_rail_align.py +++ b/tools/check_rail_align.py @@ -80,7 +80,11 @@ def main() -> int: def btn_text_x(w): """Left edge of the label: first ink past the icon's gap.""" - from PySide6.QtGui import QIcon + # Settings lays its own row out now, so read the label widget directly + # rather than hunting for a gap in the painted pixels. + lbl = getattr(win, "_nav_settings_text", None) + if w is getattr(win, "_nav_settings_btn", None) and lbl is not None: + return lbl.mapTo(rail, QPoint(0, 0)).x() if lbl.isVisible() else None if not w.text(): return None img = w.grab().toImage() diff --git a/ui/widgets.py b/ui/widgets.py index 8cfb0ce..21e3234 100644 --- a/ui/widgets.py +++ b/ui/widgets.py @@ -162,6 +162,25 @@ def guard_wheel(root: QWidget) -> None: w.installEventFilter(_wheel_guard) +def widen_popup(combo) -> None: + """Give a drop-list room for its text AND the tick beside the current item. + + macOS draws a checkmark against the selected row; Windows does not. A combo + only as wide as "VN" therefore looked fine here and had its two letters + covered there. The popup inherits the combo's width unless told otherwise, + so measure what has to fit and say so. + """ + from PySide6.QtWidgets import QStyle + + view = combo.view() + fm = view.fontMetrics() + longest = max((fm.horizontalAdvance(combo.itemText(i)) + for i in range(combo.count())), default=0) + tick = combo.style().pixelMetric(QStyle.PM_IndicatorWidth, None, combo) + pad = combo.style().pixelMetric(QStyle.PM_FocusFrameHMargin, None, combo) * 2 + view.setMinimumWidth(longest + tick + pad + 16) + + def ui_scale(widget: QWidget) -> float: """How much bigger this machine draws things than the design baseline.