<table><tr><td style="">rkflx 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/D12306">View Revision</a></tr></table><br /><div><div><p>Sorry for the back and forth, it seems this is all a bit tricky to get right…</p>

<p>Now I get a horizontal scrollbar for small window sizes and big icons (independent from the widget style in use) :(</p>

<p>The <tt style="background: #ebebeb; font-size: 13px;">5 * devicePixelRatioF</tt> sounds odd, and adding an additional <tt style="background: #ebebeb; font-size: 13px;">scrollBarWidth</tt> to <tt style="background: #ebebeb; font-size: 13px;">width</tt> looks a bit fishy, too, because this is not only an issue for 1 column of items, but happens with every width upon resize. No hacks, please (at least no ugly ones ;)</p>

<p>Let's go with the version we had before. IMO it's better to be a bit off-center for Oxygen after resizing, than to get a scrollbar everywhere and have this ugly code. I guess this has to be fixed in Oxygen eventually, and not worked around in <tt style="background: #ebebeb; font-size: 13px;">KDirOperator</tt>.</p>

<p>For the <tt style="background: #ebebeb; font-size: 13px;">+ 1</tt> in <tt style="background: #ebebeb; font-size: 13px;">const in scrollBarWidth</tt>, please move it to <tt style="background: #ebebeb; font-size: 13px;">const int viewPortWidth</tt> (i.e. it becomes <tt style="background: #ebebeb; font-size: 13px;">-1</tt> there), and as a comment add <tt style="background: #ebebeb; font-size: 13px;">Subtract 1 px to prevent flickering when resizing the window</tt>. This way future readers of the code will know what this is about. (The scrollbar width is reported correctly, so it should be left alone.)</p>

<hr class="remarkup-hr" />

<p>There is one more issue: For Oxygen, re-showing the dialog results in one column missing from displaying, it only shows up after the window has been resized. We are in a dilemma here:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">We cannot ship the file dialog without it working on Oxygen. This is no cosmetic issue, but a serious bug.</li>
<li class="remarkup-list-item">We should not add a hack (i.e. <tt style="background: #ebebeb; font-size: 13px;">5 * itemView->devicePixelRatioF()</tt>) for every style, to work around a bug which only happens with Oxygen.</li>
</ul>

<p>For now we have no choice but to add this hack (for me, 4 instead of 5 was enough). Nevertheless, please document it properly (see my suggestion below).</p>

<p>Also, please open a bug (and add a link here) or get otherwise in contact with Hugo (maintainer of Oxygen), so he can share his wisdom and give some tips.</p>

<hr class="remarkup-hr" />

<p>Patch:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="diff" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);"><span style="color: #000080">diff --git a/src/filewidgets/kdiroperator.cpp b/src/filewidgets/kdiroperator.cpp</span>
<span style="color: #000080">index db07a311..ea6373dc 100644</span>
<span style="color: #a00000">--- a/src/filewidgets/kdiroperator.cpp</span>
<span style="color: #00a000">+++ b/src/filewidgets/kdiroperator.cpp</span>
<span style="color: #800080">@@ -2589,13 +2589,16 @@ void KDirOperator::Private::updateListViewGrid()</span>
         const int height = itemView->iconSize().height() + metrics.height() * 2.5;
         const int minWidth = qMax(height, metrics.height() * 5);
 
<span style="color: #a00000">-        // this is needed for a workaround to an issue where the scrollbar area seems to be reserved at all times</span>
<span style="color: #a00000">-        const int scrollBarWidth = itemView->verticalScrollBar()->sizeHint().width() + (5 * itemView->devicePixelRatioF());</span>
<span style="color: #00a000">+        const int scrollBarWidth = itemView->verticalScrollBar()->sizeHint().width();</span>
 
<span style="color: #a00000">-        const int viewPortWidth = itemView->contentsRect().width() - scrollBarWidth;</span>
<span style="color: #00a000">+        // Subtract 1 px to prevent flickering when resizing the window</span>
<span style="color: #00a000">+        // For Oxygen a column is missing after showing the dialog without resizing it,</span>
<span style="color: #00a000">+        // therefore subtract 3 more (scaled) pixels</span>
<span style="color: #00a000">+        const int viewPortWidth = itemView->contentsRect().width() - scrollBarWidth</span>
<span style="color: #00a000">+                                  - 1 - 3 * itemView->devicePixelRatioF() ;</span>
         const int itemsInRow = qMax(1, viewPortWidth / minWidth);
         const int remainingWidth = viewPortWidth - (minWidth * itemsInRow);
<span style="color: #a00000">-        const int width = minWidth + (remainingWidth / itemsInRow) + ( itemsInRow == 1 ? scrollBarWidth : 0);</span>
<span style="color: #00a000">+        const int width = minWidth + (remainingWidth / itemsInRow);</span>
 
         const QSize itemSize(width, height);</pre></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>BRANCH</strong><div><div>master</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D12306">https://phabricator.kde.org/D12306</a></div></div><br /><div><strong>To: </strong>anemeth, Frameworks, VDG, ngraham<br /><strong>Cc: </strong>abetts, rkflx, ngraham, Frameworks, michaelh, bruns<br /></div>