Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace the alert dialog for tab title editing with popover #5790

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

liby
Copy link
Contributor

@liby liby commented Feb 15, 2025

Resolves #5768

demo.mov

During testing, it was found when window-decoration is set to none, the "Change Title..." in both the context menu and menu bar doesn't have any effect since there's no title bar to display or modify. Perhaps in this case we should hide this feature to avoid confusion.

@liby liby requested a review from a team as a code owner February 15, 2025 21:49
@mitchellh
Copy link
Contributor

During testing, it was found when window-decoration is set to none, the "Change Title..." in both the context menu and menu bar doesn't have any effect since there's no title bar to display or modify. Perhaps in this case we should hide this feature to avoid confusion.

I don't know a good way to conditionally disable this menu item. Do you know an easy way, do you want to take a stab at this? I agree with it.

@liby liby force-pushed the feature/tab-title-popover branch from fb480e2 to b5f648d Compare February 16, 2025 12:01
@liby liby force-pushed the feature/tab-title-popover branch from b5f648d to b69bf24 Compare February 16, 2025 12:09
@liby
Copy link
Contributor Author

liby commented Feb 16, 2025

I don't know a good way to conditionally disable this menu item. Do you know an easy way, do you want to take a stab at this? I agree with it.

I've handled the "Change Title..." menu item in two ways:

  1. Context menu: Hide the menu item when window-decoration is "none" or in full-screen mode (since the popover can't be positioned correctly without a title bar)
  2. Menu bar: Disable the menu item when window-decoration is "none" or in full-screen mode

Let me know if you'd like me to make any adjustments.

Screenshots image image image

@mitchellh
Copy link
Contributor

A couple things I noticed:

  • For macos-titlebar-style = tabs, I get an exception.

  • For native tabs, when the titlebar is being changed, the title turns into a ghost and we probably shouldn't do that:

CleanShot 2025-02-16 at 12 13 26@2x

  • The popover X offset is slightly off center.

@liby
Copy link
Contributor Author

liby commented Feb 17, 2025

For macos-titlebar-style = tabs, I get an exception.

I haven't been able to reproduce the exception with macos-titlebar-style = tabs.

For native tabs, when the titlebar is being changed, the title turns into a ghost and we probably shouldn't do that

This is a tricky issue I've also noticed. After investigating, it appears to be related to window focus - when the popover is shown, the main window loses focus, causing Ghostty to lose its context and fall back to the default "👻" emoji.

I've tried several approaches but haven't found a clean solution yet, as this seems to be tied to how macOS handles window focus and popover interactions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

macOS: "Change Title" feature should use a popover instead of an NSAlert
3 participants