rkflx added a comment.
Sorry for the back and forth, it seems this is all a bit tricky to get right…
Now I get a horizontal scrollbar for small window sizes and big icons
(independent from the widget style in use) :(
The `5 * devicePixelRatioF` sounds odd, and adding an additional
`scrollBarWidth` to `width` 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 ;)
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 `KDirOperator`.
For the `+ 1` in `const in scrollBarWidth`, please move it to `const int
viewPortWidth` (i.e. it becomes `-1` there), and as a comment add `Subtract 1
px to prevent flickering when resizing the window`. 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.)
---
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:
- We cannot ship the file dialog without it working on Oxygen. This is no
cosmetic issue, but a serious bug.
- We should not add a hack (i.e. `5 * itemView->devicePixelRatioF()`) for
every style, to work around a bug which only happens with Oxygen.
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).
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.
---
Patch:
diff --git a/src/filewidgets/kdiroperator.cpp
b/src/filewidgets/kdiroperator.cpp
index db07a311..ea6373dc 100644
--- a/src/filewidgets/kdiroperator.cpp
+++ b/src/filewidgets/kdiroperator.cpp
@@ -2589,13 +2589,16 @@ void KDirOperator::Private::updateListViewGrid()
const int height = itemView->iconSize().height() +
metrics.height() * 2.5;
const int minWidth = qMax(height, metrics.height() * 5);
- // this is needed for a workaround to an issue where the scrollbar
area seems to be reserved at all times
- const int scrollBarWidth =
itemView->verticalScrollBar()->sizeHint().width() + (5 *
itemView->devicePixelRatioF());
+ const int scrollBarWidth =
itemView->verticalScrollBar()->sizeHint().width();
- const int viewPortWidth = itemView->contentsRect().width() -
scrollBarWidth;
+ // Subtract 1 px to prevent flickering when resizing the window
+ // For Oxygen a column is missing after showing the dialog without
resizing it,
+ // therefore subtract 3 more (scaled) pixels
+ const int viewPortWidth = itemView->contentsRect().width() -
scrollBarWidth
+ - 1 - 3 * itemView->devicePixelRatioF() ;
const int itemsInRow = qMax(1, viewPortWidth / minWidth);
const int remainingWidth = viewPortWidth - (minWidth * itemsInRow);
- const int width = minWidth + (remainingWidth / itemsInRow) + (
itemsInRow == 1 ? scrollBarWidth : 0);
+ const int width = minWidth + (remainingWidth / itemsInRow);
const QSize itemSize(width, height);
REPOSITORY
R241 KIO
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D12306
To: anemeth, #frameworks, #vdg, ngraham
Cc: abetts, rkflx, ngraham, #frameworks, michaelh, bruns
