Skip to content

Allow tab_rmb_clicked to always work #107440

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lodetrick
Copy link
Contributor

@lodetrick lodetrick commented Jun 12, 2025

Currently, the tab_rmb_clicked signal only works when select_with_rmb is enabled. This is not always wanted however, say if you wanted to use the right-click to show a popup. This PR enables the tab_rmb_clicked signal to always be emitted if you right click a tab, even if select_with_rmb is disabled.

Old Comment

Currently, when users add a popup to a TabContainer, it only adds a small icon on the right that they can click to show the popup. Thus, a common use is to allow right clicks to tabs that also brings up the popup. However, the current method to do this is hacky and requires adding another function. The current method is to connect the tabbar's `gui_input` signal to a new method, in a line like: https://github.com/godotengine/godot/blob/d9cd011e2fa9fa9a3011371843729f33032cee35/editor/editor_dock_manager.cpp#L997

This PR adds an optional boolean to the popup method that can enable this behavior natively, reducing the amount of extra code and hacky workarounds. It's disabled by default to preserve compatibility.

I don't know how much the maintainers would want to expose this but I do think that it is a good usability enhancement. Worst case if this doesn't get exposed a boolean could be made with new methods so the existing methods don't get changed.

@kitbdev
Copy link
Contributor

kitbdev commented Jun 12, 2025

Connecting to the gui_input is very common, I wouldn't say it is hacky at all. I also don't think it is that common to have the popup be the same as the right click menu.
IMO Ideally, the dock select should only be on the right click menu since it depends on the dock, not the dock slot as a whole. But it already was at the TabContainer popup when I added it to the right click and there wasn't anything to replace it with or a good reason to remove it from there.
Changing the mouse filter based on whether this is enabled seems more hacky to me.

If anything we could improve the tab_rmb_clicked signal to not need select_with_rmb and connect to that instead.

Also this looks like it doesn't set the context dock properly, it should work even if the dock right clicked it isn't selected.

@lodetrick
Copy link
Contributor Author

I'll pivot this PR to focus on that signal then, thanks for the advice

@lodetrick lodetrick force-pushed the tabcontainer-popup branch from 4163845 to d3c8ab1 Compare June 12, 2025 23:43
@lodetrick
Copy link
Contributor Author

@kitbdev I was able to clean up the click checking, and allowed the tab_rmb_clicked to always be emitted when the right button is pressed. I assumed that the close button and right button were always contained inside the tab, but I'm pretty sure that's always true.

@lodetrick lodetrick changed the title Allow TabContainer popup to work when clicking tabs Enable tab_rmb_clicked to always work Jun 12, 2025
@lodetrick lodetrick force-pushed the tabcontainer-popup branch from 9d18202 to c2172d3 Compare June 13, 2025 01:15
@lodetrick lodetrick changed the title Enable tab_rmb_clicked to always work Allow tab_rmb_clicked to always work Jun 13, 2025
@lodetrick lodetrick force-pushed the tabcontainer-popup branch from c2172d3 to 9e1f398 Compare June 14, 2025 00:26
@Mickeon Mickeon requested a review from kitbdev June 16, 2025 20:12
@KoBeWi KoBeWi modified the milestones: 4.x, 4.6 Jun 25, 2025
@lodetrick lodetrick force-pushed the tabcontainer-popup branch from 9e1f398 to 242193e Compare June 25, 2025 18:02
@lodetrick lodetrick force-pushed the tabcontainer-popup branch from 242193e to 94f0ed0 Compare July 7, 2025 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants