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

Allow themes to override [NSMenu _isVisible] #307

Merged
merged 1 commit into from
Oct 27, 2024

Conversation

qmfrederik
Copy link
Contributor

The default implementation of [NSMenu] uses _aWindow and _bWindow to determine the visibility of the menu. Not all themes use these windows to build a menu. For example, the WinUXTheme will use Win32 APIs to build a menu.

Allow themes to override the value of [NSMenu _isVisible] by introducing a [GSTheme proposedVisibility: (BOOL)visible forMenu: (NSMenu *) menu] method. The default implementation simply returns the proposed visibility.

The default implementation of `[NSMenu]` uses `_aWindow` and `_bWindow` to determine the visibility of the menu.  Not all themes use these windows to build a menu.  For example, the WinUXTheme will use Win32 APIs to build a menu.

Allow themes to override the value of `[NSMenu _isVisible]` by introducing a `[GSTheme proposedVisibility: (BOOL)visible forMenu: (NSMenu *) menu]` method.  The default implementation simply eturns the proposed visibility.
@qmfrederik
Copy link
Contributor Author

@fredkiefer This is another approach for #304. The default implementation for [NSMenu _isVisible] is [_aWindow isVisible] || [_bWindow isVisible], but that doesn't make sense when using the WinUXTheme which doesn't use these windows.

Instead, I suggest we allow the theme to override the proposed window visibility.

Copy link
Member

@fredkiefer fredkiefer left a comment

Choose a reason for hiding this comment

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

The intention is good, but the code looks a bit ugly. As I don't have a better idea at the moment, let's merge this and think about it later again.

@fredkiefer fredkiefer merged commit acf00ce into gnustep:master Oct 27, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants