D18380: KIO: make file dialog columns resizable again (and movable)

meven edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham, #frameworks, #dolphin, apol, dfaure, ahartmetz, markg Cc: markg, cfeck, dhaumann, kwrite-devel, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham,

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > I didn't read the full encyclopedia of discussions here, I only looked at the patch. It's really a pity that you only did that now, because by now I'll have to reverse-engineer my patch (and read the encyclopedia) to remember how I arrived at the patch.

D18380: KIO: make file dialog columns resizable again (and movable)

dfaure requested changes to this revision. dfaure added a comment. I didn't read the full encyclopedia of discussions here, I only looked at the patch. But really, this is far too hacky to my taste. If QHeaderview is indeed missing a feature, it might be best to implement it there, it

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > ...aand that's exactly what happened. :( I'm not exhausted, my patch just works good enough (thanks also to feedback on here) to make me more interested in living the rest of my life rather than continue to explore alternatives. Maybe if an alternative

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham added a comment. In D18380#409963 , @ngraham wrote: > I hope not, but that's the problem with the wall-of-text communication style. Eventually people get exhausted and the discussion peters out with nothing getting done.

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > What's the minimum viable change here? I think my the implementation of my solution is pretty minimal by now, no? :) @David Faure: any ideas/suggestions, seems right up your alley? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham added a comment. I hope not, but that's the problem with the wall-of-text communication style. Eventually people get exhausted and the discussion peters out with nothing getting done. What's the minimum viable change here? REPOSITORY R241 KIO REVISION DETAIL

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. So this is just going to become a victim of the better-is-the-enemy-of-good principle? I'm not saying there can be better ways to do this but AFAIC this is already pretty good, certainly good enough and even too good in practice to keep me from more interesting

D18380: KIO: make file dialog columns resizable again (and movable)

markg added a comment. In D18380#400910 , @rjvbb wrote: > > But still, isn't there another way? Now the header and view are locked together. One doesn't work without the other. > > What's the problem with that? The custom header class isn't

D18380: KIO: make file dialog columns resizable again (and movable)

markg added a comment. In D18380#400903 , @ngraham wrote: > In D18380#400896 , @markg wrote: > > > In QHeaderView code, what we want is "QHeaderView::Interactive" [1] followed by

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > But still, isn't there another way? Now the header and view are locked together. One doesn't work without the other. What's the problem with that? The custom header class isn't public. I did indeed use it for stuff that were not part of a class, or of the

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham added a comment. In D18380#400896 , @markg wrote: > In QHeaderView code, what we want is "QHeaderView::Interactive" [1] followed by QHeaderView::resizeSection [2]. Exactly as [1] described! QHeaderView merely lacks a convenience

D18380: KIO: make file dialog columns resizable again (and movable)

markg requested changes to this revision. markg added a comment. This revision now requires changes to proceed. I too would quite like the to have resizeable columns back. But the approach you've chosen looks a little too complicated. Also, the fact that KDirOperatorDetailView::event needs

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb updated this revision to Diff 50205. rjvbb added a comment. Use qobject_cast. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18380?vs=50084=50205 REVISION DETAIL https://phabricator.kde.org/D18380 AFFECTED FILES src/filewidgets/kdiroperatordetailview.cpp To: rjvbb,

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham, #frameworks, #dolphin, apol, dfaure, ahartmetz Cc: cfeck, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > As far as I know, using qobject_cast is faster than comparing class names, because it only compares metaclass pointers. Additionally, it allows subclasses. Purely academic: that would be true for an ObjC-like construct like `isKindOfClass` but doesn't it

D18380: KIO: make file dialog columns resizable again (and movable)

cfeck added a comment. As far as I know, using qobject_cast is faster than comparing class names, because it only compares metaclass pointers. Additionally, it allows subclasses. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham, #frameworks,

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb marked an inline comment as done. rjvbb added inline comments. INLINE COMMENTS > cfeck wrote in kdiroperatordetailview.cpp:54 > Can we use qobject_cast here? Doh, of course. It's probably more expensive (which shouldn't matter here) but also means we have to include `kfilewidget.h`. Is

D18380: KIO: make file dialog columns resizable again (and movable)

cfeck added inline comments. INLINE COMMENTS > kdiroperatordetailview.cpp:54 > +} > +m_isFileWidget = pw ? strcmp(pw->metaObject()->className(), > "KFileWidget") == 0 : false; > +// install the section resize handler Can we use qobject_cast here? REPOSITORY R241 KIO

D18380: KIO: make file dialog columns resizable again (and movable)

dhaumann added a comment. I'm not against this change, but have the feeling another +2 from someone who know this stuff is a good idea. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham, #frameworks, #dolphin, apol, dfaure, ahartmetz Cc:

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. David, Andreas, any idea why the name column all of a sudden jumps to a larger width when the widget is used in a side-bar and you're making the view narrower and approach the minimum width? It works in our favour here because the end result is that the name column

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added reviewers: dfaure, ahartmetz. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham, #frameworks, #dolphin, apol, dfaure, ahartmetz Cc: kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham accepted this revision. ngraham added reviewers: Dolphin, apol. ngraham added a comment. This revision is now accepted and ready to land. In D18380#398301 , @rjvbb wrote: > > The behavior is better now, thanks. > > It's back to what

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a reviewer: Frameworks. rjvbb added a subscriber: kwrite-devel. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham, #frameworks Cc: kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > The behavior is better now, thanks. It's back to what you liked before I started tinkering with font squeezing (plus a few fixes to the behaviour in side-bars). Do you know of other applications that use this widget/mode for/in a filebrowser side-bar thingy

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham added a comment. The behavior is better now, thanks. I think it will be sufficient to fix the bug and not generate user complaints about anything! I'll let someone else do the code review. Maybe someone from #frameworks or #dolphin

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb updated this revision to Diff 50084. rjvbb added a comment. the change to the headerfile was now redundant. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18380?vs=50083=50084 REVISION DETAIL https://phabricator.kde.org/D18380 AFFECTED FILES

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb updated this revision to Diff 50083. rjvbb added a comment. Well, that was "interesting". It turns out that Qt has what looks like another path through which section sizes are calculated and through which they're shown, which apparently isn't used in file dialogs but which

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > I'm still going to try to fix this Good thing I did (am doing), because what you call the jarring exists also without font stretching. It's something in Qt that somehow doesn't occur in file dialogs but only with applications of the widget like in Kate. It is

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. Yikes, and I can reproduce that. Did you notice this with previous versions that used font stretch? (Probably not if stretch had no effect for the font(s) you tried it with...) I'm still going to try to fix this; even if in the end it doesn't go in there's be a

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Sorry, I really don't think this text squeezing feature is actually desirable. Here's how it works in Kate: F6561155: squeezed-2019-01-21_16.03.33.webm

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb updated this revision to Diff 50032. rjvbb added a comment. This version uses letterspacing rather than font stretch, so works regardless of whether a stylename has been set on the font. Letterspacing reduction has a stronger impact on readability than stretch (if you cannot know

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > The discussion in that bug kind of petered out, unfortunately. It did, and indeed. I thought a fix had been implemented for KFontRequester, but either I'm mistaken, or I have a fix locally. > I think you're the only one who understands what's going on there,

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham added a comment. If this code depends on https://bugs.kde.org/show_bug.cgi?id=378523 being fixed, then we need to fix that first. The discussion in that bug kind of petered out, unfortunately. I think you're the only one who understands what's going on there, so would you care to

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. Oh sh@@t, I understand what's happening here. The default look-and-feel still appends the Regular stylename to the font, and since font stretch is a programmatic way to set a font style ("condensed") this too is overridden when a stylename is set. That does

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > I don't see the squeezed text when using Breeze and Noto Sans. Am I missing something? No, I don't think so. This must correspond to the 2nd pair of images I attached. > I kind of hope so, because I predict that if we ship with text that gets squeezed

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham added a comment. I don't see the squeezed text when using Breeze and Noto Sans. Am I missing something? I kind of hope so, because I predict that if we ship with text that gets squeezed at various sizes, we'll totally get bug reports about it. :-) No need to overcomplicate

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. My QtCurve-based theme, "narrow mode" just activated: F6558463: kdialog-narrow-mode.png Idem, "narrow mode" just deactivated F6558464: kdialog-regular-mode.png Default theme, narrow

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb updated this revision to Diff 49947. rjvbb added a comment. Final attempt at being a bit clever :) Entering "narrow mode" now also sets the font stretch (exitting the mode will reset the stretch), the same as the Mac Finder will do. I use factor 83, which corresponds roughly to

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. One thing we might be able to do is use QFont::setStretch() to make the text a bit more compact. I'll follow that thought for a bit. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham Cc: kde-frameworks-devel, michaelh,

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. I agree that reducing the size column width isn't as easy as I thought. I find it takes up more space than necessary when horizontal space is at a premium and I was hoping to get a more compact read-out that can still be interpreted ... and that would show itself in

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham added a comment. This version works pretty well! I'm not sure I like the bit where you make the Size column 20% smaller under certain circumstances though. Since the Size column was previously sized perfectly, making it 20% smaller makes all of its items get elided and become

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > If there's still enough room to show everything without a horizontal scrollbar, then we're still good. No, resizing a view so that it becomes taller than wide shouldn't make it behave as if it's become very narrow all of a sudden. Similarly, a very narrow

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb updated this revision to Diff 49908. rjvbb added a comment. tentative fix for "very narrow views". This imposes a simple minimum width on the name column. The value is taken from (33% wider than) the date column which has a useful width (which is already based on the current

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham added a comment. In D18380#396578 , @rjvbb wrote: > > Maybe we can incorporate some more intelligence here > > Same commit? It is a separate issue in a sense, no - you say "it *still* starts" so not a regression I introduced?

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb added a comment. > Maybe we can incorporate some more intelligence here Same commit? It is a separate issue in a sense, no - you say "it *still* starts" so not a regression I introduced? > and set a sensible default width for the Name column when the height of the view is

D18380: KIO: make file dialog columns resizable again (and movable)

ngraham added a comment. This does indeed make the Name column resizable again (+1) but when the view is very narrow, it still starts out with a poor size by default. Maybe we can incorporate some more intelligence here and set a sensible default width for the Name column when the height of

D18380: KIO: make file dialog columns resizable again (and movable)

rjvbb retitled this revision from "KIO: make file dialog columns resizable again" to "KIO: make file dialog columns resizable again (and movable)". rjvbb set the repository for this revision to R241 KIO. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18380 To: rjvbb,