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

HIG-compliant for GTK menus #1931

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

MuhammadMuradG
Copy link
Contributor

@MuhammadMuradG MuhammadMuradG commented May 6, 2023

Describe your changes in detail

This PR convert the discussion into #874 to an actual implementation after formalize it.

By default and for any app with gtk-bacend, this PR will:

  1. Remove ToolBar and MenuBar.
  2. Add a HeaderBar for app.
  3. The HeaderBar will have Control buttons (Close, Maximize and Minimize) on the right side of it.
  4. A Primary menu, with "open-menu-symbolic", is the default menu.
  5. This Primary menu placed at right side of HeaderBar on the left of Control buttons.
  6. This Primary menu contains from bottom the following items (About app-name, Help, Keyboard Shortcuts, Preferences, a horizontal Separator).
  7. App name will be putted on the center of the HeaderBar.
  8. Any user defined menu will be placed at left on the HeaderBar.
  9. The user defined menu will be a widget of Gtk.DropDown.
  10. These list of user defined menus will be ended with a vertical separator.
  11. The items in ToolBar will be placed at left of the HeaderBar after vertical separator.
  12. The commands in HeaderBar, were previously placed in the ToolBar, will not be added automatically to Primary menu.

What problem does this change solve?

This PR will make the gtk-backend follow the HIG-compliant of GTK menus.

Related issues:

Refs #874

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@freakboy3742
Copy link
Member

This looks like a pretty good summary of #874; two details that stood out:

  1. This Primary menu contains from bottom the following items (About app-name, Help, Keyboard Shortcuts, Preferences, a horizontal Separator).

Might want to clarify the exact ordering here - it reads as if "horizontal separator" will be the last item, which doesn't make much sense :-) These groupings are important, as they they influence where users can insert their own app-level commands.

  1. The commands in HeaderBar, were previously placed in the ToolBar, will not be added automatically to Primary menu.

This isn't something we discussed explicitly - I'm guessing the intention is that GTK considers the menu and toolbar to be a singular unit, so if someone has explicitly promoted an item to the toolbar, that supersedes the default presentation in a menu? It's inconsistent with the implementation on other platforms, but I guess it's consistent with the spirit of GTK's HIG; we'll need to make sure this "inconsistency" is flagged in documentation.

One additional item that isn't mentioned here is the implementation of the Application menu (the one in the bar at the top of the screen, not the top of the window), which only seems to have "New Window", "Preferences" and "Quit" - and probably also has an interaction with DocumentApp. I'm not sure if you want to consider that in-scope for this PR - I'd be entirely comfortable with leaving that for a separate PR if you wanted to.

@MuhammadMuradG
Copy link
Contributor Author

MuhammadMuradG commented May 8, 2023

Might want to clarify the exact ordering here - it reads as if "horizontal separator" will be the last item, which doesn't make much sense :-) These groupings are important, as they they influence where users can insert their own app-level commands.

After I read it again, look likes it needs some paraphrasing, and it is as following: The items on the menu are arranged from bottom of the menu to top as following the last item is 'About app-name', followed by 'Help', 'Keyboard Shortcuts', 'Preferences', and finally, a horizontal separator."

This isn't something we discussed explicitly - I'm guessing the intention is that GTK considers the menu and toolbar to be a singular unit, so if someone has explicitly promoted an item to the toolbar, that supersedes the default presentation in a menu? It's inconsistent with the implementation on other platforms, but I guess it's consistent with the spirit of GTK's HIG; we'll need to make sure this "inconsistency" is flagged in documentation.

That's good that you alerted me to that, I'll make sure I add that to the documentation.

One additional item that isn't mentioned here is the implementation of the Application menu (the one in the bar at the top of the screen, not the top of the window), which only seems to have "New Window", "Preferences" and "Quit" - and probably also has an interaction with DocumentApp. I'm not sure if you want to consider that in-scope for this PR - I'd be entirely comfortable with leaving that for a separate PR if you wanted to.

As you say, I think it will be better to keep it for a separate PR.

@freakboy3742
Copy link
Member

After I read it again, look likes it needs some paraphrasing, and it is as following: The items on the menu are arranged from bottom of the menu to top as following the last item is 'About app-name', followed by 'Help', 'Keyboard Shortcuts', 'Preferences', and finally, a horizontal separator."

Ok ... but that means the first item is a horizontal separator. What is it... separating? What goes above the separator? I feel like I'm missing something here.

@MuhammadMuradG
Copy link
Contributor Author

Yes, it is a separator. It is a separator between those items and other user-defined items. If the user does not add any other items, it will not be exist.

@freakboy3742
Copy link
Member

Yes, it is a separator. It is a separator between those items and other user-defined items. If the user does not add any other items, it will not be exist.

Ah - so, that can be addressed by setting those 4 items in a common section with section=sys.maxsize. You can see this in practice in the Cocoa backend. There's no explicit "insert separator"; instead there's 4 sections in Cocoa's menu - About, , Preferences, , Hide app, Hide others, Show all, Quit. The use of sections automates the introduction of separators, and by using sys.maxsize, the section is guaranteed to be last in the ordering.

@MuhammadMuradG
Copy link
Contributor Author

This is actually what I did in a last commit 😄.

@freakboy3742
Copy link
Member

This is actually what I did in a last commit 😄.

Similar, but not quite (at least, based on what you've pushed to Github) - you've used order, not section. Order can also useful (although insertion order will be honoured by default, so it's probably overkill in this case); it's section that forces the introduction of the separator.

@MuhammadMuradG MuhammadMuradG mentioned this pull request Jun 12, 2023
42 tasks
@mhsmith
Copy link
Member

mhsmith commented Oct 2, 2023

@MuhammadMouradG: You've done 10 consecutive merges from main in the last 5 months without making any other changes. Do you plan to continue working on this PR?

@MuhammadMuradG
Copy link
Contributor Author

@mhsmith: Yes, I plan to continue working on this PR. I'm waiting for two things to be completed before I can make further progress:

I'm doing these consecutive merges to announcing that the PR is still active and keeping things updated to early observe and resolving any raising conflicts. If these consecutive merges raising any problem to Toga team please let me know.

@mhsmith
Copy link
Member

mhsmith commented Oct 2, 2023

No problem, thanks for the explanation.

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

Successfully merging this pull request may close these issues.

3 participants