[KFileWidget] Hide places frame and header
ClosedPublic

Authored by broulik on Feb 5 2018, 3:19 PM.

Details

Summary

This avoids a double "Places" header and also gets rid of the superfluous frame around the widget, similar to Dolphin. It has never been possible to detach the dock from the window anyway.
The close button is also removed as a side-effect of this but should a user really want to hide it, this can still be done by pressing F9 or in the menu. Resizing the places list is still possible.

Test Plan

Before

After

Breeze default


Breeze with borders enabled

Oxygen default

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Feb 5 2018, 3:19 PM
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptFeb 5 2018, 3:19 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Feb 5 2018, 3:19 PM
ngraham added a subscriber: ngraham.EditedFeb 5 2018, 3:33 PM

Shouldn't this be controlled by System Settings > Application Style > Widget Style > Breeze > Configure > Frames > Draw Frame around Dockable panels? Otherwise, people who check that setting will have a visible frame around the Places panel in Dolphin, but not in open/save file pickers, and will wonder why and file bugs about it.

Shouldn't this be controlled by System Settings > Application Style > Widget Style > Breeze > Configure > Frames > Draw Frame around Dockable panels?

If that option is enabled you get a frame even with this patch. Otherwise, the default, is nice and clean.

markg added a subscriber: markg.Feb 5 2018, 3:41 PM

Why does it show the panel as if it were unlocked?
Your before image looks exactly like an unlocked frame in Dolphin.

Would it be possible to show it as if it were locked? That would solve all the issues with it, right?

ngraham accepted this revision as: ngraham.Feb 5 2018, 11:28 PM
This revision is now accepted and ready to land.Feb 5 2018, 11:28 PM
mart accepted this revision.Feb 6 2018, 10:15 AM

Would it be possible to show it as if it were locked? That would solve all the issues with it, right?

I don't get it. That "lock" feature is entirely a Dolphin invention. It does exactly what I do here:

void DolphinDockWidget::setLocked(bool lock)
{
    ...
        if (lock) {
            ...
            setTitleBarWidget(m_dockTitleBar);
            setFeatures(QDockWidget::NoDockWidgetFeatures);

with m_dockTitleBar being a custom widget for some added padding

broulik updated this revision to Diff 26642.Feb 6 2018, 12:57 PM
broulik retitled this revision from RFC: [KFileWidget] Hide places frame and header to [KFileWidget] Hide places frame and header.
broulik edited the test plan for this revision. (Show Details)
  • Add custom widget for added spacing, fixes the items glued to the top when borders are enabled, see updated Test Plan for new screenshots
markg added a comment.EditedFeb 6 2018, 12:57 PM

Would it be possible to show it as if it were locked? That would solve all the issues with it, right?

I don't get it. That "lock" feature is entirely a Dolphin invention. It does exactly what I do here:

void DolphinDockWidget::setLocked(bool lock)
{
    ...
        if (lock) {
            ...
            setTitleBarWidget(m_dockTitleBar);
            setFeatures(QDockWidget::NoDockWidgetFeatures);

with m_dockTitleBar being a custom widget for some added padding

Looks like i was looking at the wrong picture. I was looking at your after image and comparing that to the "locked" state in dolphin.
The image i was expecting is the one you call "crammed at the top" :)

Imho, the "crammed at the top" version looks best as the "after" one just has some weird empty room above the panel now. But feel free to use the one you think fits best.

A suggestion though if you do choose for the "after" version. Would it be possible to rearrange the layout then?
So:

  • move the actions to the top, right above the panel.
  • move the location bar next to the actions

I think that would look nice :)

apol added a subscriber: apol.Feb 6 2018, 3:32 PM
apol added inline comments.
src/filewidgets/kfilewidget.cpp
1359–1360

Does it really make sense that it's a dock if it can't be interacted with?

broulik added inline comments.Feb 13 2018, 12:59 PM
src/filewidgets/kfilewidget.cpp
1359–1360

I don't know. You can still resize the panel, I also wanted to keep the patch as unintrusive as possible.

If no Frameworks dev objects within next few days this will be pushed

This revision was automatically updated to reflect the committed changes.