<table><tr><td style="">rjvbb added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D5990" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D5990#112211" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D5990#112211</a>, <a href="https://phabricator.kde.org/p/elvisangelaccio/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@elvisangelaccio</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>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...).</p></div>
</blockquote>
<p>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.</p>
<p>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.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D5990#inline-24548" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">main.cpp:162</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">KAboutData</span><span style="color: #aa2211">::</span><span class="n">setApplicationData</span><span class="p">(</span><span class="n">aboutData</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">application</span><span class="p">.</span><span class="n">setWindowIcon</span><span class="p">(</span><span class="n">QIcon</span><span style="color: #aa2211">::</span><span class="n">fromTheme</span><span class="p">(</span><span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"ark"</span><span class="p">)));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">application</span><span class="p">.</span><span class="n">setWindowIcon</span><span class="p">(</span><span class="n">QIcon</span><span style="color: #aa2211">::</span><span class="n">fromTheme</span><span class="p">(</span><span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"ark"</span><span class="p">)<span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">application</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">windowIcon</span></span><span class="bright"></span><span class="p"><span class="bright">()</span>));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D5990#inline-24549" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">main.cpp:331-332</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Q_UNUSED</span><span class="p">(</span><span class="n">openFileEventHandler</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">// we don't provide a fullscreen mode</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">window</span><span style="color: #aa2211">-></span><span class="n">setWindowFlags</span><span class="p">(</span><span class="n">window</span><span style="color: #aa2211">-></span><span class="n">windowFlags</span><span class="p">()</span> <span style="color: #aa2211">&</span> <span style="color: #aa2211">~</span><span class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">WindowFullscreenButtonHint</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#endif</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.)<br />
The zoom/maximise button is not affected.</p>
<p style="padding: 0; margin: 8px;">The <tt style="background: #ebebeb; font-size: 13px;">WindowFullscreenButtonHint</tt> 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.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R36 Ark</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D5990" rel="noreferrer">https://phabricator.kde.org/D5990</a></div></div><br /><div><strong>To: </strong>rjvbb, Ark, KDE Applications<br /><strong>Cc: </strong>elvisangelaccio, kde-utils-devel, kde-mac, tctara<br /></div>