Skip to content

Honor macOS title bar double-click setting - #76

Open
klopez4212 wants to merge 1 commit into
mainfrom
kennylopez-macos-titlebar-double-click
Open

klopez4212 wants to merge 1 commit into
mainfrom
kennylopez-macos-titlebar-double-click

Conversation

@klopez4212

Copy link
Copy Markdown
Contributor

Summary

  • follow the macOS title-bar double-click setting
  • preserve dragging and non-macOS behavior
  • cover pointer and native preference handling

Validation

  • focused TypeScript and Rust tests
  • build, design browser checks, Clippy
  • full scan: 389/390 browser journeys; the lone WebKit notification timeout reproduces on untouched origin/main
  • native tests: 12/13; the terminal environment test is blocked by the machine ClickFix acknowledgement prompt
  • staged native app on the Builderlab relay

Signed-off-by: klopez4212 <klopez4212@gmail.com>
@klopez4212
klopez4212 requested review from a team, comp615 and wesbillman as code owners September 15, 2026 17:07
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-15T17:12:04.214925Z 74c120c PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 74c120cf0c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@@ -0,0 +1,64 @@
import type { MouseEventHandler } from "react";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add the required Signed-off-by trailer

The reviewed commit has no Signed-off-by trailer, so it violates the repository's DCO requirement and will fail the hosted DCO Check. Ensure this commit carries the verified author's sign-off while preserving its actual authorship before integration.

AGENTS.md reference: AGENTS.md:L123-L126

Useful? React with 👍 / 👎.

Comment on lines +45 to +46
start?.x !== event.clientX ||
start.y !== event.clientY

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow normal pointer slop on the second click

When the pointer moves by even one CSS pixel between the second mouse-down and mouse-up, these exact coordinate comparisons suppress the configured title-bar action even though event.detail === 2 shows that the platform accepted the interaction as a double-click. Normal hand jitter therefore makes Fill/Zoom or Minimize unreliable; use a small drag threshold or native double-click detection rather than requiring perfectly stationary coordinates.

Useful? React with 👍 / 👎.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head 74c120cf0c84f3513e6ff1cf0a7a7d3309347baf against base 42e92280dc6abb173b62d882df81d9f4400664e4. Required contract: honor macOS title-bar double-click preferences while preserving dragging, clickable header controls and non-macOS behavior.

P1: Gate the macOS action definitions so Linux and Windows pass their required native checks

src-tauri/src/lib.rs:20-33 adds TitleBarDoubleClickAction and title_bar_double_click_action unconditionally, but their only production use is inside #[cfg(target_os = "macos")]. Both existing non-macOS Clippy jobs therefore fail with dead_code promoted to errors. This is a PR-introduced cross-platform integration failure, not just a missing green badge: both native pipelines stop before their tests, and required CI fails.

Exact-head evidence: Linux job 104483479543 reports the unused enum at line 21 and function at line 27 under cargo clippy --workspace --locked --all-targets -- -D warnings; Windows job 104483479829 reports the same errors under the package Clippy command.

Smallest repair: gate both declarations with #[cfg(any(target_os = "macos", test))] or an equivalent narrow platform boundary so the pure tests still compile cross-platform. Keep warnings enforced. Exit criterion: both existing native jobs pass on the repaired head, including their currently skipped tests/tool integration.

Reviewed behavior and evidence limits

The frontend preserves primary/direct-background targeting and installs the replacement only on macOS desktop; web and non-macOS keep the existing drag-region attributes. The existing pointer-slop bot comment is not adopted as a new regression: lock-matched Tauri 2.11.5's previous window/scripts/drag.js already requires identical second-down/up coordinates. The sole PR commit contains a sign-off and hosted DCO Check succeeds, contrary to the older bot metadata comment.

Independent native review confirms the same platform-gating defect and narrow repair. The command re-reads the preference on each invocation, dispatches through the caller-injected window and keeps the Objective-C dependency macOS-only. Fresh/unset-preference OS behavior was not established in this source-only review; that uncertainty is not promoted into another blocker.

Existing CI run 34999321580 is completed/failure at this exact head. JavaScript, browser measurements and all Chromium/WebKit shards succeeded; Linux/Windows lint and the required aggregate failed as described above. Source/metadata review on the authorized laptop only: no checkout, installation, build, test execution, CI rerun or native-window acceptance. The author's separate local browser/PTY limitations are not the cause of these hosted failures.

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.

2 participants