[vlc-devel] [PATCH] macosx: First commit VLCStatusBarIcon code for macosx gui.

[David Fuhrmann]

> Am 02.01.2016 um 12:04 schrieb User Goran Dokic <vlc at 8hz.com>:
> 
> Hi all,
> 
> Here's the first version of my macosx VLCStatusBarIcon (statusitem)
> addition and changes to the latest master branch for integration. I 
> find it comfy to have it. 
> 
> There are 2 new objective c files, two very minor changes to existing
> code, 5 new small PNGs (icons) and dirs, a new xib and some small
> additions to the project file and 2 Makefile.am files.
> 
> Main functions:
> - Menubar status icon for macosx for vlc
> - Playback controls synced to VLC internals (follow app state)
>    - Play/Pause 
>    - Stop
>    - Next
>    - Previous
>    - Toggle Random
> - Utility menu items/functions
>    - Show (part of) URL/PATH and Copy URL/Path to clipboard
>    - Restore MainWindow to front (from any state)
> - Tooltip with some info, to display current track/time/url/path 
> - Seems to work for video too (bonus)
> 
> Design choice.. Tooltip and menu URL item are updated with a TimerHandler.. 
> I'd wanted to do it on demand, but I can't catch the mouse in/out 
> events without some major trickery. Neither does the tooltip signal its
> impending creation/display, like the menu fortunately does (menuNeedsUpdate), 
> afaict. The impact is small though, so maybe later.
> 
> I'm afraid I have no way of testing this on other macs than my retina
> probook with 10.10.5. Seems to work well for what I wanted out of it.
> 
> Awaiting your response,
> 
> Regards,
> 
> 	woran

Hi Goran,

First, thanks for your contribution. I like the idea. The patch needs some improvements still, but I would be glad if we can merge your code in the end, after we improved it a bit.

I’ll do a more detailed review later one, but first some general remarks:
- The first menu item looks a bit ugly, with those dashes if nothing is playing, and with the complete url if something is playing. Maybe we should discuss about the usefulness of this feature (who needs the complete url, which also starts with file:// for local media? How do you use this information? Maybe some other media summary is better?)
- Activating the main window: should be renamed a bit, but might be useful indeed.
- Play / Stop: Fine, but I do not like the icons that much. I think we should remove them, as this would be a more OS X like menu.
- Rest of the menu seems fine.
- Tooltip: I would remove the URL from the list. Also, if nothing is playing, I would just write something like „No media playing currently“, not listing all those items explicitly.

Regarding the gathering of all information:
Currently you are using a poll-based approach and the timer to update the tooltip periodically. I understand this is more easy at a first glance, but it is not ok for the final version.
Polling is bad in general, and more importantly, information is always a bit out of date (I already saw the URL in the first menu item missing quite some times, even after the media started playing already).
I think we need to implement a direct update of the tooltip once information changes. But we can help you with that.

Best regards,
David