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

Make the use of ellipses in menus consistent #89796

Closed
wants to merge 1 commit into from

Conversation

timothyqiu
Copy link
Member

This PR removes ellipses when the menu item...

  • does not require additional information:
    • executes immediately: Insert Key (Animation track editor)
    • brings settings dialog: Editor Settings, Configure Snap, Edit, etc
    • shows a separate tool/view: Command Palette, Manage Export Templates, Orphan Resource Explorer, About Godot, etc
  • embeds submenu: Export As, Set Folder Color, etc

Also added ellipses for...

  • delete actions that require user confirmation
  • rename actions (even when it's in-place rename as users can still press ESC to cancel)

References about the use of ellipses in menu items:

Indicate a command that needs additional information (including a confirmation) by adding an ellipsis at the end of the label.
This doesn't mean you should use an ellipsis whenever an action displays another window only when additional information is required to perform the action. For example, the commands About, Advanced, Help, Options, Properties, and Settings must display another window when clicked, but don't require additional information from the user. Therefore they don't need ellipses. 1

Append an ellipsis to a menu item’s label when people need to provide additional information before the action can complete. The ellipsis character (…) signals that another view will open in which people can input information or make choices. 2

Footnotes

  1. https://learn.microsoft.com/en-us/windows/win32/uxguide/cmd-menus#using-ellipses

  2. https://developer.apple.com/design/human-interface-guidelines/menus

@timothyqiu timothyqiu added bug topic:editor usability topic:gui cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Mar 23, 2024
@timothyqiu timothyqiu added this to the 4.3 milestone Mar 23, 2024
@timothyqiu timothyqiu requested review from a team as code owners March 23, 2024 07:59
@AThousandShips
Copy link
Member

AThousandShips commented Mar 23, 2024

I'd say this isn't extremely clear cut, many well established programs use ellipses where you have removed them:

  • Export as (gimp, open-office)
  • Settings, in various phrasings (open-office, Kate, octave, KeePass, GH Desktop, Signal, Visual Studio, Kdnlive)
  • Configure grid/snap/etc. (gimp)
  • Help Search (Kdnlive)

And surely others, so I don't think just going by a design document is necessarily what to follow

A few of these were also added recently so let's get a clear decision before we keep going back and forth undoing changes :)

I'd label this as an enhancement and not a bug as well

@AThousandShips
Copy link
Member

AThousandShips commented Mar 23, 2024

My opinion is that what's more important than following some third party design guides is to have a uniform design language, where specific terms or symbols mean specific things, here we have used the ellipses to mean "you need to respond to this in some way", and that's important IMO

Now for "About" it makes sense, but generally I'd say the ellipses are appropriate where they trigger some kind of dialog, or more specifically perhaps, it depends on the question "if I press this, is the operation done?", i.e. does something happen and complete by pressing it, not opening a dialog, or requiring a response, a complete action

@@ -2854,7 +2854,7 @@ void AnimationTrackEdit::gui_input(const Ref<InputEvent> &p_event) {
bool selected = _try_select_at_ui_pos(pos, mb->is_command_or_control_pressed() || mb->is_shift_pressed(), false);

menu->clear();
menu->add_icon_item(get_editor_theme_icon(SNAME("Key")), TTR("Insert Key..."), MENU_KEY_INSERT);
menu->add_icon_item(get_editor_theme_icon(SNAME("Key")), TTR("Insert Key"), MENU_KEY_INSERT);
Copy link
Member

Choose a reason for hiding this comment

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

I'd say this should be dependent on the track type, some tracks do spawn a menu, like method tracks

Copy link
Member Author

Choose a reason for hiding this comment

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

This may be the same as "Save", which conditionally pops up a dialog. It does not use ellipses conventionally.

But anyway, adjusting the text based on whether user interaction is needed is always an option :P

Copy link
Member

Choose a reason for hiding this comment

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

I'd say it's different as it's not conditional on if the file is already saved, but on the track type, but good point

@timothyqiu timothyqiu modified the milestones: 4.3, 4.x May 24, 2024
@timothyqiu timothyqiu removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label May 24, 2024
@timothyqiu timothyqiu force-pushed the ellipses branch 2 times, most recently from de1ed17 to e2aea17 Compare June 1, 2024 02:10
@timothyqiu
Copy link
Member Author

Closing. It's too exhausting to come back from time to time to check if a PR from half a year ago needs to be rebased.

@timothyqiu timothyqiu closed this Sep 25, 2024
@timothyqiu timothyqiu deleted the ellipses branch September 25, 2024 01:30
@timothyqiu timothyqiu removed this from the 4.x milestone Sep 25, 2024
@timothyqiu timothyqiu removed request for a team September 25, 2024 01:31
@timothyqiu timothyqiu removed the request for review from a team September 25, 2024 01:31
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.

2 participants