Skip to content

Add application menu on Windows - #174

Merged
fluiddot merged 5 commits into
trunkfrom
add/windows-app-menu
May 28, 2024
Merged

Add application menu on Windows#174
fluiddot merged 5 commits into
trunkfrom
add/windows-app-menu

Conversation

@fluiddot

@fluiddot fluiddot commented May 27, 2024

Copy link
Copy Markdown
Contributor

Related to 7084-gh-Automattic/dotcom-forge.

Proposed Changes

  • Separate menu creation from setupMenu function to a new function getAppMenu. This function will be used when setting up the application menu on macOS and when displaying the app menu as a context menu on Windows.
  • Create popupMenu to display the app menu as a context menu.
  • Add button to WindowsTitlebar component. This button will trigger the context menu.
App Open app menu

Testing Instructions

Windows

  • Open the app.
  • Observe that in the title bar there's a button on the top-left corner.
  • Click on the menu button.
  • Observe a context menu is displayed with the same items as the application menu on macOS.
  • Click on some if the items to check the menu works as expected.

macOS

  • Open the app.
  • Observe the application menu is displayed as in previous versions.
  • Observe no menu button is displayed in the title bar.
  • Click on some if the items to check the menu works as expected.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@fluiddot fluiddot self-assigned this May 27, 2024
@fluiddot fluiddot changed the title Add app's menu on Windows May 27, 2024
@fluiddot
fluiddot marked this pull request as ready for review May 27, 2024 17:30
@fluiddot
fluiddot requested review from a team May 27, 2024 17:30

@derekblank derekblank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. I observed from the testing instructions that the application menu is added and available on Windows, with no change to the macOS behavior.

windows application menu
@wojtekn

wojtekn commented May 28, 2024

Copy link
Copy Markdown
Contributor

@fluiddot thanks, menu still works correctly on macOS and works fine on Windows. It's a significant gain as now Windows users can:

  • Open About Studio
  • Trigger Check for Updates
  • Open 'Add Site' and Settings forms

I noticed some minor glitches:

  • 'Studio -> Services' and 'Studio -> Hide Studio' do nothing
  • 'File -> Close Window' exists from the app instead of closing the window
  • accelerators (keyboard shortcuts) don't work
  • The Edit menu looks like not needed on Windows - those actions can target form text fields, but when any field is visible, the menu can't be accessed.

However, we can tackle those under separate issues/PRs.

@fluiddot

Copy link
Copy Markdown
Contributor Author

I noticed some minor glitches:

  • 'Studio -> Services' and 'Studio -> Hide Studio' do nothing
  • 'File -> Close Window' exists from the app instead of closing the window
  • accelerators (keyboard shortcuts) don't work
  • The Edit menu looks like not needed on Windows - those actions can target form text fields, but when any field is visible, the menu can't be accessed.

However, we can tackle those under separate issues/PRs.

Good catch @wojtekn. I'll tweak the menu items based on the platform, in the case of Windows some of them won't be included.

@fluiddot

Copy link
Copy Markdown
Contributor Author

I've updated the app menu item for Windows in bc373f1.

The following items/menus have been removed:

  • Studio/Services: This only applies to macOS.
  • Studio/Hide: This only applies to macOS.
  • File/Close Window: Closes the app instead of the current window.
  • Edit: Seems only relevant for macOS:
  • Window: Seems only relevant for macOS:

@wojtekn wojtekn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Window: Seems only relevant for macOS:

Actually, Window -> Minimize worked as expected on Windows. The Window -> Zoom did nothing.

@fluiddot

Copy link
Copy Markdown
Contributor Author

Actually, Window -> Minimize worked as expected on Windows. The Window -> Zoom did nothing.

Yep, I also noticed that when testing. We could keep the Windows menu with only the Minimize item, but I found it redundant as we have the minimize button in the title bar.

@fluiddot
fluiddot merged commit 2c3069b into trunk May 28, 2024
@fluiddot
fluiddot deleted the add/windows-app-menu branch May 28, 2024 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants