D12306: Filepicker dialog proper grid icon layout

[Henrik Fehlauer]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180419/6ba45d3c/attachment-0001.html>