D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added a task: T8552: Polish Open/Save dialogs. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure, #frameworks, broulik Cc: #dolphin, cfeck, fabiank, broulik, michaelh, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure, #frameworks, broulik Cc: #dolphin, cfeck, fabiank, broulik, michaelh, ngraham, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham accepted this revision. ngraham added a comment. Works, thanks! REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure, #frameworks, broulik Cc: #dolphin, cfeck, fabiank, broulik, michaelh, ngraham, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey marked 2 inline comments as done. sharvey added a comment. Merge errors tracked down and eradicated. INLINE COMMENTS > ngraham wrote in kdiroperatordetailview.cpp:53 > Uh-oh :) What is that? (finding it and killing it) REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey updated this revision to Diff 32002. sharvey added a comment. - Repairing merge error REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11993?vs=31992=32002 BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 AFFECTED FILES

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added inline comments. INLINE COMMENTS > kdiroperatordetailview.cpp:53 > > +<<< HEAD > void KDirOperatorDetailView::setModel(QAbstractItemModel *model) Uh-oh :) > kdiroperatordetailview.cpp:71 > +=== > +>>> Remove unneeded resize code; ensure other columns fill to

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey added a comment. Rebased (I think). Had to manually resolve a conflict. Hopefully it'll land now. REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure, #frameworks, broulik Cc: #dolphin, cfeck, fabiank,

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey updated this revision to Diff 31992. sharvey marked an inline comment as done. sharvey added a comment. - Remove unneeded resize code; ensure other columns fill to contents - Remove leftover commented-out code; restore variable declaration REPOSITORY R241 KIO CHANGES SINCE LAST

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added a comment. @sharvey, can you re-base this onto current master? It doesn't want to land. REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure, #frameworks, broulik Cc: #dolphin, cfeck, fabiank,

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added a comment. In D11993#244077 , @broulik wrote: > The only downside this has is that you can no longer resize the column wider manually but then you might as well resize the window and actually //see// more :) Precisely! :)

D11993: Tweak column widths in tree view of file open/save dialogs

broulik accepted this revision. broulik added a comment. The only downside this has is that you can longer resize the column wider manually but then you might as well resize the window and actually //see// more :) REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added a comment. Any more opinions on this? As you can see from the Summary section, it fixes (or obviates) a //ton// of bugs, many of them very old. REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure,

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham edited the summary of this revision. REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure, #frameworks Cc: #dolphin, cfeck, fabiank, broulik, michaelh, ngraham, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added a reviewer: Frameworks. ngraham edited subscribers, added: Dolphin; removed: Frameworks. REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure, #frameworks Cc: #dolphin, cfeck, fabiank, broulik, michaelh,

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham edited the summary of this revision. REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure Cc: cfeck, fabiank, broulik, #frameworks, michaelh, ngraham, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham edited the summary of this revision. REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure Cc: cfeck, fabiank, broulik, #frameworks, michaelh, ngraham, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham edited the summary of this revision. REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure Cc: cfeck, fabiank, broulik, #frameworks, michaelh, ngraham, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added a comment. In D11993#241607 , @cfeck wrote: > I vaguely remember there was some code to delay resizing of the columns until all items were loaded for performance reasons. Please check the commit history before deleting code.

D11993: Tweak column widths in tree view of file open/save dialogs

cfeck added a comment. I vaguely remember there was some code to delay resizing of the columns until all items were loaded for performance reasons. Please check the commit history before deleting code. REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham accepted this revision. REPOSITORY R241 KIO BRANCH arcpatch-D11993 REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure Cc: fabiank, broulik, #frameworks, michaelh, ngraham, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey marked 3 inline comments as done. sharvey added inline comments. INLINE COMMENTS > ngraham wrote in kdiroperatordetailview_p.h:56 > Don't leave code commented out like this; if it's not used anymore, delete it! Remnants of testing. Won't do that again. REPOSITORY R241 KIO BRANCH

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey updated this revision to Diff 31545. sharvey added a comment. - Remove leftover commented-out code; restore variable declaration REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11993?vs=31540=31545 BRANCH arcpatch-D11993 REVISION DETAIL

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added a comment. Got a few code niggles, detailed below. Function-wise, this is a vast usability improvement. And in case you have a really excessively long file name, you can just use Tree view (the non-details version), which only has one column. INLINE COMMENTS >

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey added a comment. Also, yes - Stretch has disabled the ability to manually resize the columns. Columns 1 & 2 are set to fit the size of their contents, while column 0 fills all the remaining space. Resizing the dialog results in only the Name column (column 0) to change size; the

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey added a comment. All seems to be well with the removal of the extraneous resizing code. Here's the result of a test with a ridiculously long filename: F5796698: Screenshot_20180406_195204.png I think that looks perfectly reasonable.

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey updated this revision to Diff 31540. sharvey added a comment. - Remove unneeded resize code; ensure other columns fill to contents REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11993?vs=31517=31540 BRANCH arcpatch-D11993 REVISION DETAIL

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey added a comment. I'll give that a test. I'm sure I can find a file (or make one up) with an absurdly long filename. Once I figure out why @ngraham's uploaded diff won't apply... grr. REPOSITORY R241 KIO BRANCH column-widths (branched from master) REVISION DETAIL

D11993: Tweak column widths in tree view of file open/save dialogs

fabiank added a comment. I'm to tired to test this myself, but could you please test this with the edge case where some file names are so long that they don't fit in the view? Also, Stretch disables manual resizing, right?' If that is also deemed useful, we could adapt the solution from

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey added a comment. In the works... REPOSITORY R241 KIO BRANCH column-widths (branched from master) REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure Cc: broulik, #frameworks, michaelh, ngraham, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added a comment. And while we're digging into this code, it would be quite nice to set `QHeaderView::ResizeToContents` for the other two columns so they always display the correct width and never have to be resized. That even fits under the umbrella of the patch's title "tweak

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added a comment. I wasn't able to find any regressions stemming from removing that code, and once gone, I was able to hack out even more code that seemed to exist purely in support of it. Here's a diff that applies on top of this patch: https://paste.kde.org/p9z7riqdo REPOSITORY

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey added a comment. Give me a moment to comment it out again and re-check. I thought it was, but let me run another test. REPOSITORY R241 KIO BRANCH column-widths (branched from master) REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure Cc: broulik,

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham added a comment. Question: is `expandNameColumn()` even needed anymore with this change? REPOSITORY R241 KIO BRANCH column-widths (branched from master) REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure Cc: broulik, #frameworks, michaelh,

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey marked an inline comment as done. sharvey added a comment. In D11993#241387 , @ngraham wrote: > This patch is a breath of fresh air when using Detailed View or Detailed Tree View. You can resize the window and open and close the preview

D11993: Tweak column widths in tree view of file open/save dialogs

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. So. Much. Better. Now. This patch is a breath of fresh air when using Detailed View or Detailed Tree View. You can resize the window and open and close the preview without wanting

D11993: Tweak column widths in tree view of file open/save dialogs

broulik added a comment. Thanks for updating! I think this patch makes sense (and seems to work well) but I'm not too familiar with `QTreeView`, let's wait a bit for further feedback and merge it once Frameworks 5.45 has been tagged. REPOSITORY R241 KIO REVISION DETAIL

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey marked an inline comment as done. sharvey added inline comments. INLINE COMMENTS > broulik wrote in kdiroperatordetailview.cpp:103 > For first colum you want to set it to `0`, here it blew up the "Size" column > to full width I got misled by my manual adjustment of the columns from

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey updated this revision to Diff 31517. sharvey added a comment. - Fix incorrect column index REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11993?vs=31516=31517 BRANCH column-widths (branched from master) REVISION DETAIL

D11993: Tweak column widths in tree view of file open/save dialogs

broulik added inline comments. INLINE COMMENTS > kdiroperatordetailview.cpp:103 > QHeaderView *headerView = header(); > -headerView->setSectionResizeMode(QHeaderView::Interactive); > +headerView->setSectionResizeMode(1, QHeaderView::Stretch); >

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey added a comment. F5795938: Screenshot_20180406_145748.png Before (from user bug report) F5795942: Screenshot_20180406_145619.png After (mine) REPOSITORY R241 KIO REVISION DETAIL

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure Cc: #frameworks, michaelh, ngraham, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey edited the summary of this revision. sharvey added reviewers: ngraham, dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D11993 To: sharvey, ngraham, dfaure Cc: #frameworks, michaelh, ngraham, bruns

D11993: Tweak column widths in tree view of file open/save dialogs

sharvey created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. sharvey requested review of this revision. REVISION SUMMARY Adjust column sizing options so that "last" column is no longer the largest. First column