D5990: Ark: Mac adaptations

René J.V. Bertin noreply at phabricator.kde.org
Sun May 28 07:39:51 UTC 2017


rjvbb added a comment.


  In https://phabricator.kde.org/D5990#112211, @elvisangelaccio wrote:
  
  > Yes, please (phabricator allows a single review for a branch with more than one commits, but the UI doesn't let you see which commit does what...).
  
  
  Either way I don't submit off locally committed changes. I've added inline explanations to the 2 small changes to main.cpp that I would have committed directly if there hadn't been other changes too. I think there isn't much to review about them but let me know if you want to do a full review (or 2!) anyway. If not that saves all of us some work.
  
  Also, please let me know if you think I should reduce the #ifdefs to the strict minimum in main.cpp . A priori none of them is required, even the event filter could be installed as-is on all supported platforms if you don't mind invoking such a filter for nothing.

INLINE COMMENTS

> main.cpp:162
>      KAboutData::setApplicationData(aboutData);
> -    application.setWindowIcon(QIcon::fromTheme(QStringLiteral("ark")));
> +    application.setWindowIcon(QIcon::fromTheme(QStringLiteral("ark"), application.windowIcon()));
>  

Specify the current application icon as the fallback for the lookup from theme. It will be empty like the default fallback when there is no embedded app icon and so doesn't change anything. Same thing on platforms where there is an embedded icon but no support for icon themes.

> main.cpp:331-332
> +            Q_UNUSED(openFileEventHandler);
> +            // we don't provide a fullscreen mode
> +            window->setWindowFlags(window->windowFlags() & ~Qt::WindowFullscreenButtonHint);
> +#endif

This removes the fullscreen button in the window titlebar. It is a bit cumbersome to exit the resulting fullscreen mode when there is no menu action of keyboard shortcut to accomplish this - which I think must be because there is little need for such a mode in an application like Ark. (Especially not knowing that the native Mac fullscreen mode can black out all other monitors on a multi-head setup.)
The zoom/maximise button is not affected.

The  `WindowFullscreenButtonHint` flag is Mac-specific as far as I know and thus doesn't require an #ifdef. The QOpenFileEvent handler stuff doesn't need it either.

REPOSITORY
  R36 Ark

REVISION DETAIL
  https://phabricator.kde.org/D5990

To: rjvbb, #ark, #kde_applications
Cc: elvisangelaccio, kde-utils-devel, kde-mac, tctara
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20170528/39a2613d/attachment.html>


More information about the kde-mac mailing list