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

Add editor setting for AcceptDialog OK/Cancel buttons positioning #59382

Merged

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Mar 21, 2022

The position (left/right) of the Cancel and OK buttons in AcceptDialog
are DisplayServer specific, as Windows uses OK/Cancel and macOS uses Cancel/OK.

Linux/X11 currently uses the macOS convention which is also the GTK+/GNOME one,
though it's not consistent with Qt/KDE applications which follow the Windows
convention.

Since that can't satisfy everyone, it's best if it's configurable also for the
editor (it's already configurable for the project).

Fixes #59379.

image

@bruvzg I used "OK First" instead of "OK Left" assuming that this would be handled with UI mirroring too for RTL, but it doesn't seem to be (at least from a quick look at scene/gui/dialogs.cpp). I don't know if it should be, we'd have to check e.g. Arabic on Windows and macOS.

Also, I find this whole swap_cancel_ok API in AcceptDialog a bit hacky (and badly named as it doesn't tell users swapping from what to what, i.e. that the "reference" unswapped positions are the GNOME/macOS ones). And on Linux one could make a case to have the behavior DE-dependent so that on predominantly Qt-based KDE it would follow the Windows convention... but that's a mess (and as a KDE user I wouldn't like the behavior to change now that I'm used to OK on the right).

@akien-mga akien-mga added enhancement topic:editor usability cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Mar 21, 2022
@akien-mga akien-mga added this to the 4.0 milestone Mar 21, 2022
@bruvzg
Copy link
Member

bruvzg commented Mar 21, 2022

I used "OK First" instead of "OK Left" assuming that this would be handled with UI mirroring too for RTL, but it doesn't seem to be (at least from a quick look at scene/gui/dialogs.cpp). I don't know if it should be, we'd have to check e.g. Arabic on Windows and macOS.

There's no need for extra code in the scene/gui/dialogs.cpp, RTL layout will mirror the underlying box container.

@akien-mga
Copy link
Member Author

As discussed in PR review meeting, I'll update this to also change the project setting to use an enum like done here for the editor setting.

Might also research whether we should auto-detect the preferred style on X11.

@akien-mga akien-mga modified the milestones: 4.0, 4.x Oct 5, 2022
@akien-mga akien-mga force-pushed the editor-acceptdialog-swap-cancel-ok branch from 6955810 to 90be1af Compare October 5, 2022 14:59
@akien-mga
Copy link
Member Author

I'm not finding time to rework this and handle the project setting, which would turn out to be a big rabbit hole as there's a lot of inconsistencies in our handling of OK/Cancel to handle to do this right.

So I think this could be merged as it improves the situation for the editor somewhat, which is the main Godot project where the position of such buttons matters and might need to be customized by the user.

@akien-mga akien-mga modified the milestones: 4.x, 4.0 Oct 5, 2022
The position (left/right) of the Cancel and OK buttons in AcceptDialog
are DisplayServer specific, as Windows uses OK/Cancel and macOS uses Cancel/OK.

Linux/X11 currently uses the macOS convention which is also the GTK+/GNOME one,
though it's not consistent with Qt/KDE applications which follow the Windows
convention.

Since that can't satisfy everyone, it's best if it's configurable also for the
editor (it's already configurable for the project).

Fixes godotengine#59379.
@akien-mga akien-mga force-pushed the editor-acceptdialog-swap-cancel-ok branch from 90be1af to 9bb05de Compare October 5, 2022 15:51
@akien-mga akien-mga requested a review from a team as a code owner October 5, 2022 15:51
@akien-mga akien-mga merged commit 3a2e749 into godotengine:master Oct 11, 2022
@akien-mga akien-mga deleted the editor-acceptdialog-swap-cancel-ok branch October 11, 2022 10:34
@timothyqiu
Copy link
Member

Cherry-picked for 3.6.

@timothyqiu timothyqiu removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Dec 1, 2022
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.

OK and Cancel buttons are swapped between Windows and Linux
3 participants