Review Request 127706: adaptations for OS X

[Emmanuel Pescosta]

> On April 28, 2016, 11:35 a.m., Emmanuel Pescosta wrote:
> > src/dolphinmainwindow.cpp, lines 170-177
> > <https://git.reviewboard.kde.org/r/127706/diff/1/?file=456420#file456420line170>
> >
> >     Wouldn't it be better to just disable the ShowMenubar action and maybe force menuBar()->setVisible(true) on OS X?
> >     
> >     This would reduce the number of required changes and avoid that showMenu is still toggle-able.
> >     
> >     What do you think?
> 
> René J.V. Bertin wrote:
>     Well, that could be a solution too. I went with this approach because it allows to preserve the usual behaviour when I start the application with the XCB QPA (= under X11). That isn't really a frequent use-case of course.

> I went with this approach because it allows to preserve the usual behaviour when I start the application with the XCB QPA (= under X11)

This would also be possible with my approach. Just use your canHideMenuBar() to set the visibility of the menu bar and to disable the showMenu action. ;)


- Emmanuel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127706/#review94949
-----------------------------------------------------------


On April 22, 2016, 2:40 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127706/
> -----------------------------------------------------------
> 
> (Updated April 22, 2016, 2:40 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: dolphin
> 
> 
> Description
> -------
> 
> Is there any incentive to support Dolphin on OS X (and MS Windows)?
> 
> If so, this patch introduces 2 changes:
> 
> - the application icon isn't replaced unconditionally by the result of QIcon::fromTheme() but instead that call receives the current application icon as a fallback. This avoids removing the existing application icon in cases where icon themes are not functional (default on OS X and MS Windows).
> 
> - deactivates the control menu button in the toolbar in case the menubar is always present (= on OS X). This prevents continuous error messages about menu items being reassigned, due to reparenting QActionWidgets (= using a single instance in multiple menus). It also avoids a crash-on-exit that seems to be related to QActionWidget reuse (double free somewhere deep Qt).
> 
> As to adding an application icon on OS X (and presumably MS Windows): this seems no longer possible from an installed icon theme but apparently requires shipping the icons nowadays. Using an installed icon theme requires an additional step to set up properly named symlinks (in the build directory; icon file names are required to contain their resolution in their name). Once that is done, the following patch takes care of the rest:
> 
> ```
> --- src/orig.CMakeLists.txt	2016-04-13 18:49:42.000000000 +0200
> +++ src/CMakeLists.txt	2016-04-21 17:08:56.000000000 +0200
> @@ -1,3 +1,4 @@
> +include(ECMAddAppIcon)
>  
>  configure_file(config-baloo.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/config-baloo.h)
>  
> @@ -270,6 +271,10 @@
>  # TODO Does anything replace kde4_add_app_icon ?
>  #kde4_add_app_icon(dolphin_SRCS "${KDE4_ICON_INSTALL_DIR}/oxygen/*/apps/system-file-manager.png")
>  
> +# Sets the icon on Windows and OSX
> +file(GLOB ICONS_SRCS "${CMAKE_CURRENT_BINARY_DIR}/icons/*system-file-manager.png")
> +ecm_add_app_icon(dolphin_SRCS ICONS ${ICONS_SRCS})
> +
>  kf5_add_kdeinit_executable(dolphin ${dolphin_SRCS})
>  
>  target_include_directories(kdeinit_dolphin PRIVATE ${PHONON_INCLUDES})
> ```
> 
> 
> Diffs
> -----
> 
>   src/dolphinmainwindow.cpp d4f2b06 
>   src/main.cpp 0bbae97 
> 
> Diff: https://git.reviewboard.kde.org/r/127706/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.9.5 with Qt 5.6.0, FWs 5.20.0 and dependencies installed in /opt/local
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20160607/642bebce/attachment.htm>