munki / munki

Managed software installation for macOS —
https://www.munki.org/munki/
Other
3.1k stars 345 forks source link

Munki 2 - Main menu text not clickable only icons #381

Closed natewalck closed 10 years ago

natewalck commented 10 years ago

From yves.zie...@googlemail.com on August 30, 2014 00:46:18

Not very important, but sometimes i click and nothing happens, because i clicked the text under the main icons.

Enhancement is to make a clickable square around the main icons and text.

Original issue: http://code.google.com/p/munki/issues/detail?id=381

natewalck commented 10 years ago

From gregnea...@mac.com on September 01, 2014 07:21:14

This looks like it will be very difficult to fix.

Originally Managed Software Center had a "fake" toolbar with normal buttons at the top-center. When this caused problems when translating the labels in to other languages, I switched to a real NSToolbar, which automatically resizes items to display the full label.

But in order to get the icons to display properly (correct size, blue highlight when clicked) I needed to use NSButtons instead of simple toolbar images. It appears that once you use anything other than an image for a toolbar item that the label text does not respond to clicks.

Therefore fixing this behavior may be difficult.

natewalck commented 10 years ago

From danielha...@gmail.com on September 01, 2014 13:10:00

How do you feel about calling private APIs? If you don't plan to ever put this in the App Store then it shouldn't be a problem. If Apple ever changes the API it should just stop working (the method would never be called in the first place).

I have this fixed in a test program by overloading the NSButton hitTest: method to include the adjacent label view and by overloading the NSButtonCell _hitTestForTrackMouseEvent:inRect:ofView: (this is the private API one) to again include the adjacent label view. This seems to produce reliable results in all my click testing.

If you don't have any qualms about overloading the private API to append new behavior to it I will submit a pull request. I still have to see about converting the .m subclass files to .py (or does that matter?)

natewalck commented 10 years ago

From danielha...@gmail.com on September 01, 2014 13:11:33

It looks, by the way, that when you use a custom view / control all mouse events get passed to the control rather than handled internally by the toolbar. Thus, the control knows nothing about a label and just ignores any mouse events that are not inside it's own rect frame.

natewalck commented 10 years ago

From gregnea...@mac.com on September 02, 2014 09:55:35

I wish clicking on the label was also forwarded to the control, but it appears not to be so.

Apple is clearly doing something very custom in their App Store toolbar, which of course, they are free to do.

I'd like to see your test program demonstrating your solution to the issue, or a pull request so I can play. But wondering why you needed to override the private _hitTestForTrackMouseEvent:inRect:ofView: method instead of the public hitTestForTrackMouseEvent:inRect:ofView: method..?

natewalck commented 10 years ago

From danielha...@gmail.com on September 02, 2014 13:24:01

There isn't a public hitTestForTrackMouseEvent:inRect:ofView:, just a generic hitTestForEvent:inRect:ofView: which never gets called in this case. If there is a public method I can't find it in the docs, and Xcode also complains about it not existing.

I haven't had time to re-work the code into python form (it may be easier than I expect), but here is the test project attached. The real code is in the MUToolbarButton and MUToolbarButtonCell files. The only change to the stock toolbar settings in "interface builder" was to change the class of the NSButton and NSButtonCell to use the custom classes.

The only artifact I noticed visually is that the text itself doesn't highlight the same way it does on a standard image toolbar button, but I almost didn't notice until I was a few inches away from the screen.

natewalck commented 10 years ago

From gregnea...@mac.com on September 02, 2014 13:40:19

"The only artifact I noticed visually is that the text itself doesn't highlight the same way it does on a standard image toolbar button" in App Store, the label text doesn't highlight at all when clicked.

natewalck commented 10 years ago

From danielha...@gmail.com on September 02, 2014 19:09:23

Never noticed, good to know.

I could never get PyObjC to call the private method of the superclass. It let me override the method, but not call the super so I had to slightly change the way the NSButtonCell subclass works, but everything still seems pretty much to work the same. Not sure how google code handles pull requests, but here is the commit to my clone: https://code.google.com/r/danielhazelbaker-munki/source/detail?r=1e7503d046309d8d8ebcc25baf7bb7b0cb1a5651&name=Munki2

natewalck commented 10 years ago

From gregnea...@mac.com on September 02, 2014 20:06:28

"I could never get PyObjC to call the private method of the superclass" I found the same today when testing. I could call public methods of the superclass, but the PyObjC bridge complained about the private method. I'll be interested to see how your workaround compares to the things I tried.

Note that we can just use the original Objective-C implementations; they work as-is.

i need to test this on all the supported OSes (10.6-10.10). I'm uncomfortable relying on private methods, but it seems necessary to get the desired behavior. Given the other issues with the toolbar under Yosemite, though, I worry that this is all house of cards ready to collapse, as we hack around trying to duplicate the non-standard things Apple is doing.

natewalck commented 10 years ago

From gregnea...@mac.com on September 02, 2014 22:09:13

Fix merged here: https://code.google.com/p/munki/source/detail?r=1e7503d046309d8d8ebcc25baf7bb7b0cb1a5651&name=Munki2

Status: Started

natewalck commented 10 years ago

From gregnea...@mac.com on September 04, 2014 13:47:53

I'm calling this resolved.

Status: Fixed