Skip to content

Conversation

nshcr
Copy link
Contributor

@nshcr nshcr commented Oct 7, 2025

It turns out that the traffic light position setup was only executed after the main window was created, so the cloned project windows didn't get the same configuration.

pub fn open_project_in_window(handle: tauri::AppHandle, id: ProjectId) -> Result<(), Error> {
let label = std::time::UNIX_EPOCH
.elapsed()
.or_else(|_| std::time::UNIX_EPOCH.duration_since(std::time::SystemTime::now()))
.map(|d| d.as_millis().to_string())
.context("didn't manage to get any time-based unique ID")?;
window::create(&handle, &label, id.to_string()).map_err(anyhow::Error::from)?;
Ok(())
}

I believe moving the related code to window::create should fix this issue.
There's a comment in the original code saying "NOTE: Make sure you only call this ONCE per window." — this may not actually be necessary, since moving it into window::create should ensure it only runs once per window creation anyway.

Additionally, due to a Tauri bug (tauri-apps/tauri#13044), the traffic light position resets after setting a window title.
However, the position setup code can be reapplied after the window is resized.
To demonstrate that this fix works, I tested it by changing the window size and observing whether the traffic light position is properly set again.

Screen.Recording.2025-10-08.at.05.38.13.mov
Screen.Recording.2025-10-08.at.05.39.42.mov

I also noticed that the traffic lights appear slightly larger on my build.
I'm not sure what causes this — it might be related to my local build, since even after reverting all changes, the buttons are still larger.

If they also appear larger on your build, it might be worth slightly adjusting the inset values to center them better:

// from
window.setup_traffic_lights_inset(LogicalPosition::new(16.0, 25.0))?;
// to
window.setup_traffic_lights_inset(LogicalPosition::new(16.0, 28.0))?;

If this style change isn't desirable or if there's a flaw in my logic, please let me know how I can improve it.
You can also feel free to close this PR — but note that it's a dependency for my next PR (#10574), so I'll need to adjust that one if this gets closed.

Copy link

vercel bot commented Oct 7, 2025

@nshcr is attempting to deploy a commit to the GitButler Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions bot added the rust Pull requests that update Rust code label Oct 7, 2025
@Byron
Copy link
Collaborator

Byron commented Oct 8, 2025

Thanks so much for getting this started.

Let me first acknowledge that currently both in Nightly and here the traffic lights aren't properly positioned once the window comes up due to tauri-apps/tauri#13044.

Screen.Recording.2025-10-08.at.04.43.36.mov

So that's not a regression.

Also, for unknown reason, this PR gives us properly sized traffic lights.

The picture below shows the traffic lights with the position adjusted as proposed here, in comparison with the Web Inspector of the same dev build, and with Nightly (greyed out traffic lights).

Screenshot 2025-10-08 at 04 55 05

We see that the new size is default OS. Since we never did anything to make them smaller, I think this PR is a net-win to get us to look more like a standard MacOS window in that regard.

It's also worth to highlight that with the adjusted positioning, the bigger traffic lights look quite right.

Screenshot 2025-10-08 at 04 58 12

However, I'd think @PavelLaptev may want to double-check this.

Lastly, with this PR applied, new windows will have a proper traffic light location. What's visible here is that Nightly doesn't adjust the traffic light position in new windows, which is fixed by this PR.

Screen.Recording.2025-10-08.at.05.00.44.mov

All this considered, I think this PR should be merged as it's a net win which fixes a couple of bugs, including the traffic light size.

As a follow-up, I could imagine the setWindowTitle function in the frontend to be changed so that it directs this to the backend, which can then update the traffic lights right away.

@Byron Byron enabled auto-merge October 8, 2025 03:07
@Byron
Copy link
Collaborator

Byron commented Oct 8, 2025

I have created #10575 for a quick stab at getting the traffic lights right after a label change.

@Byron Byron force-pushed the fix-traffic-lights-inset branch from e2f525f to 5357fa8 Compare October 8, 2025 03:21
@Byron Byron merged commit e8680ff into gitbutlerapp:master Oct 8, 2025
19 of 20 checks passed
@Byron
Copy link
Collaborator

Byron commented Oct 8, 2025

I tried, but it doesn't look like there is a good workaround for the traffic-light positioning issue, unfortunately.

@nshcr
Copy link
Contributor Author

nshcr commented Oct 8, 2025

@Byron Thank you so much for the detailed response to this PR, and for merging it!

Also, for unknown reason, this PR gives us properly sized traffic lights.

From what I've observed, both sizes of traffic lights exist across various macOS apps, including both system and third-party ones.

Apps using small traffic lights include:

  • IntelliJ IDEA
  • VS Code
  • ChatGPT
  • Apple Numbers
Screenshot 2025-10-08 at 12 19 50

Apps using large traffic lights include:

  • Chrome
  • Arc
  • Safari
  • Apple Notes
Screenshot 2025-10-08 at 12 18 53

So… I honestly have no idea what's going on with macOS.

I tried, but it doesn't look like there is a good workaround for the traffic-light positioning issue, unfortunately.

I actually spent a whole day debugging this with Claude Sonnet 4.5 and GPT-5 Codex, I tried modifying the tauri_plugin_trafficlights_positioner dependency directly, adding native delegate event listeners, using Tauri tao APIs, and even calling native APIs via objc2.
None of it worked, and I have to admit my Rust skills just aren't strong enough to fix this one.

As a follow-up, I could imagine the setWindowTitle function in the frontend to be changed so that it directs this to the backend, which can then update the traffic lights right away.

That's definitely an interesting direction to explore.
However, considering the extra complexity that workaround code might introduce, I think it's probably best to just wait for the upstream to fix this bug, or maybe we could simply stop setting window titles (just kidding :P).

@Byron
Copy link
Collaborator

Byron commented Oct 8, 2025

That's definitely an interesting direction to explore.
However, considering the extra complexity that workaround code might introduce, I think it's probably best to just wait for the upstream to fix this bug, or maybe we could simply stop setting window titles (just kidding :P).

I did try that in #10575, but it didn't work. It good to know that you already dug much deeper than that and also couldn't get it to work. So it's probably best to wait for a fix, or get Electron ready ;).

@nshcr
Copy link
Contributor Author

nshcr commented Oct 9, 2025

@Byron I realized could the larger traffic lights be because we're both building on macOS 26, while the CI image is running macOS 15?

So I waited for the next nightly build to see how the traffic lights look there, and it turns out that might be the case.

Screenshot 2025-10-09 at 14 34 24

If the final packaged build ends up still with the smaller traffic lights, we'd probably better revert this position change.

nshcr added a commit to nshcr/gitbutler that referenced this pull request Oct 9, 2025
@Byron
Copy link
Collaborator

Byron commented Oct 9, 2025

That's a great finding, thanks so much!

#10586 fixes this so official builds will look Ok in future, but the underlying problem is still not solved. We consider to not meddle with the lights anymore and make the design work around them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants