Skip to content

Show enabled/disabled state on Enable Terminal Usage menu item - #724

Merged
hannesa2 merged 1 commit into
masterfrom
feature/cli-tool-menu-indicator
Sep 26, 2026
Merged

Show enabled/disabled state on Enable Terminal Usage menu item#724
hannesa2 merged 1 commit into
masterfrom
feature/cli-tool-menu-indicator

Conversation

@hannesa2

@hannesa2 hannesa2 commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Add -isCliToolInstalled to ApplicationController, which checks whether /usr/local/bin/gitx is a symlink pointing at the gitx tool bundled in this app. -validateMenuItem: uses this to set the checkmark state on the installCliTool: menu item, so users can tell at a glance whether the command line tool is already enabled for this copy of GitX.

Now you see if terminal usage is already enabled
image

@hannesa2

hannesa2 commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

I moved the icon in the same column of the other icons
image

@goneng

goneng commented Sep 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@hannesa2, showing whether terminal usage is enabled is a nice touch.
Two thoughts on the how:

  • The standard way to show this is the menu item's checkmark.
    The HIG uses checkmarks for toggled items (Menus > Toggled items), and
    icons are for identifying actions.
    Setting menuItem.state from isCliToolInstalled, as changeAppearance:
    already does, would work the same on every macOS version.
  • Matching the macOS 26 icon column may not pay off for long.
    Apple has stepped back from icons on menu items:
    in macOS 27, NSMenu hides symbol images by default
    (release notes, via Michael Tsai), and the HIG now asks for
    images only on the most important actions.
    So checkmark.circle.fill would most likely stop showing on 27, and
    on macOS 11-15, which have no icon column, it shows
    as a filled circle next to the title, which is non-standard.

@hannesa2
hannesa2 force-pushed the feature/cli-tool-menu-indicator branch from 7b8395b to f018388 Compare September 25, 2026 21:25
goneng
goneng approved these changes Sep 25, 2026
View reviewed changes

@goneng goneng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thanks @hannesa2. Could you squash the three commits into one before merging? The middle one adds the icon that the last one removes.

@hannesa2
Show enabled/disabled state on Enable Terminal Usage menu item
151db63 
Add -isCliToolInstalled to ApplicationController, which checks whether
/usr/local/bin/gitx is a symlink pointing at the gitx tool bundled in
this app. -validateMenuItem: uses this to set the checkmark state on
the installCliTool: menu item, so users can tell at a glance whether
the command line tool is already enabled for this copy of GitX.

Move icon to same column of the other icons

Use menu item checkmark state instead of a custom icon for CLI tool status
@hannesa2
hannesa2 force-pushed the feature/cli-tool-menu-indicator branch from f018388 to 151db63 Compare September 26, 2026 05:06
@hannesa2

hannesa2 commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Sure, I squashed all three, reverted to menuItem.state for the checkmark, matching changeAppearance:'s pattern and the HIG guidance from the review — no more custom SF Symbol image, so it's version-portable and won't disappear on macOS 27.

It looks now like the first image
image

@hannesa2
hannesa2 requested a review from goneng September 26, 2026 05:12
@hannesa2

hannesa2 commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Screenshot differs: emulatorApi=arm64 with 2 screenshot(s)

setpoint (old) | diff | actual screenshot (new)

commit-context-menu.pngscreenshot

history-view.pngscreenshot

goneng
goneng approved these changes Sep 26, 2026
View reviewed changes

@goneng goneng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks - I love it

@hannesa2
hannesa2 merged commit 2e3d439 into master Sep 26, 2026
3 checks passed
@hannesa2
hannesa2 deleted the feature/cli-tool-menu-indicator branch September 26, 2026 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@goneng goneng goneng approved these changes

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hannesa2 @goneng