Add functionality for docks locking
ClosedPublic

Authored by antonanikin on Oct 6 2016, 9:41 AM.

Details

Summary

This patch provides mechanism for docks locking. This can be useful for some users and allows us to lock dock state during call Hide/Restore Docks action. Such locked docks don't change it's visible state during call. This provides behavior similar to QtCreator - we can lock for example left dock (with Project tool view) and quickly hide all other docks when it needed (after hiding unlocked docks the editor will be focused).

The patch allows user to control which docks should be locked by adding Lock the Panel from Hiding item to dock panel context menu. Lock settings are saved in global configuration file.

Test Plan

Tested with master branch.

Diff Detail

Repository
R33 KDevPlatform
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
antonanikin updated this revision to Diff 7140.Oct 6 2016, 9:41 AM
antonanikin retitled this revision from to QtCreator-style "Focus Editor or Hide Docks".
antonanikin updated this object.
antonanikin edited the test plan for this revision. (Show Details)
antonanikin added a reviewer: KDevelop.
antonanikin set the repository for this revision to R33 KDevPlatform.
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptOct 6 2016, 9:41 AM
antonanikin updated this revision to Diff 7141.Oct 6 2016, 9:45 AM
apol added a subscriber: apol.Oct 6 2016, 7:31 PM

I'm not convinced we want this. When would you lock a side?

I'm not convinced we want this.

This functionality is an "additional", it doesn't replace old actions. We also can disable it by default:

  • set empty default shortcut
  • or add checkbox to GUI configuration page which totally blocks this approach (if it blocked we also don't create lock buttons).

When would you lock a side?

Current version lock only left side by default.

apol added a comment.Oct 7 2016, 2:51 PM

Why would I want to lock the left side? Maybe we want to lock the project view?

Don't you think that we should look into simplifying the paradigm instead of adding more components to it?

Why would I want to lock the left side? Maybe we want to lock the project view?

I thought about this. But we usually "lock" position of some Tool View to specific Dock (side). At this point of view it is reasonable to add only one lock button (or something other mechanism) for whole Dock (panel) instead adding such functionality to every Tool View.

Don't you think that we should look into simplifying the paradigm instead of adding more components to it?

Reasonable simplifying is already good, you are right. But I think this patch does not increases complexity of current paradigm. And I also don't know how to provide more simple way to operating current Tool Views, sorry. Can you share your ideas about this?

antonanikin updated this object.
antonanikin edited the test plan for this revision. (Show Details)

Update to master, add GUI to enable/disable feature

@apol, hi.

What are you think about last revision? Now we have this feature disabled by default - this don't change our "classic" behavior.

apol added a comment.Oct 24 2016, 12:10 PM

I'd say we need to get this one right, otherwise we'll end up with something weird. As I see it there's 2 changes here and I'd like to have them separately:

  • View shortcuts toggle: 2 states (visible, hidden) instead of 3 (visible, focused, hidden).
    • you call that QtCreator style. I'd say we can find a more meaningful name than that.
    • would it maybe make sense to just make the 2 states the new default? We can have some discussion WRT how people should give a toolview the focus and what are the use-cases.
  • Locking sides
    • I see why you want it but I really think it's a corner use-case. I haven't seen a bug report complaining about it. I suggest moving the action to the context menu at least, so we can have some text explaining what it does and so it doesn't add clutter.
antonanikin updated this revision to Diff 7714.Oct 28 2016, 5:56 AM

update to master

  • you call that QtCreator style. I'd say we can find a more meaningful name than that.

Ok, I will think about new name. Suggestions are welcome :)

  • View shortcuts toggle: 2 states (visible, hidden) instead of 3 (visible, focused, hidden).
    • would it maybe make sense to just make the 2 states the new default? We can have some discussion WRT how people should give a toolview the focus and what are the use-cases.

Maybe I misunderstood you, correct me please. I think we should have 3 states:

  1. focused - when we open some tool view (with shortcut or "by hand" (mouse click)) we should focus it to do something inside. For example if we show context help it seems to be useful to focus help page for scroll, find, etc with keyboard.
  1. unfocused - when we return to editor, tool view lost focus, but don't closed.
  1. hidden - when we use some mechanism for close tool view.

New behavior uses all 3 states. If we focused inside some tool view and run new "focus or hide" action we go to the editor and tool view will be unfocused. Second action run will close (hide) all unlocked tool views.

  • I see why you want it but I really think it's a corner use-case. I haven't seen a bug report complaining about it. I suggest moving the action to the context menu at least, so we can have some text explaining what it does and so it doesn't add clutter.

Ok, but for which element we should add such context menu item? Adding it for each tool view seems to be an "overhead" and produce user's confusing. Adding it to whole dock can leads to situation, when we have too many toolviews on some side, which hides dock panel and also "hides" panel's context menu. Maybe we should adds such settings to KDevelop config page? I think "blocking/unblocking" sides is not so frequent operation and this way may be preferable.

apol added a comment.Oct 28 2016, 2:47 PM
  • you call that QtCreator style. I'd say we can find a more meaningful name than that.

Ok, I will think about new name. Suggestions are welcome :)

  • View shortcuts toggle: 2 states (visible, hidden) instead of 3 (visible, focused, hidden).
    • would it maybe make sense to just make the 2 states the new default? We can have some discussion WRT how people should give a toolview the focus and what are the use-cases.

Maybe I misunderstood you, correct me please. I think we should have 3 states:

  1. focused - when we open some tool view (with shortcut or "by hand" (mouse click)) we should focus it to do something inside. For example if we show context help it seems to be useful to focus help page for scroll, find, etc with keyboard.
  2. unfocused - when we return to editor, tool view lost focus, but don't closed.
  3. hidden - when we use some mechanism for close tool view.

    New behavior uses all 3 states. If we focused inside some tool view and run new "focus or hide" action we go to the editor and tool view will be unfocused. Second action run will close (hide) all unlocked tool views.

Focus and unfocused are the same state WRT toolview toggling.

  • I see why you want it but I really think it's a corner use-case. I haven't seen a bug report complaining about it. I suggest moving the action to the context menu at least, so we can have some text explaining what it does and so it doesn't add clutter.

Ok, but for which element we should add such context menu item? Adding it for each tool view seems to be an "overhead" and produce user's confusing. Adding it to whole dock can leads to situation, when we have too many toolviews on some side, which hides dock panel and also "hides" panel's context menu. Maybe we should adds such settings to KDevelop config page? I think "blocking/unblocking" sides is not so frequent operation and this way may be preferable.

I think it's better in the toolbar context menu. In the settings page we'll lose the context.

Focus and unfocused are the same state WRT toolview toggling.

In this case we will have "focus editor AND close tool views" (needs 1 action run) instead previous "focus editor OR close tool views" (needs 2 action runs) - it's right?

I think it's better in the toolbar context menu. In the settings page we'll lose the context.

Ok.

antonanikin updated this revision to Diff 7797.Nov 1 2016, 8:56 AM
  • Reset UI settings to master
  • Remove lock button
  • Add context menu item to the dock bar
  • Fix logic: focusEditor OR HideUnlockedDocks --> focusEditor AND HideUnlockedDocks
antonanikin retitled this revision from QtCreator-style "Focus Editor or Hide Docks" to Add new action "Focus Editor and Hide Docks".Nov 1 2016, 9:01 AM
antonanikin updated this object.

@apol, hi.

I fix the patch for your suggestions, check it please.

apol added inline comments.Nov 4 2016, 2:11 PM
sublime/mainwindow_p.cpp
117 ↗(On Diff #7797)

What's the difference between this action and "Hide/Restore Docks"? They feel really similar, too similar.

antonanikin added inline comments.Nov 4 2016, 2:35 PM
sublime/mainwindow_p.cpp
117 ↗(On Diff #7797)

We have 2 old actions:

  1. Focus Editor
  2. Hide/Restore Docks.

I don't change them and anything other to keep "old-style" behavior.

The patch adds new action which merges presented 2 "old" parts into simple "chain" for usability improvement. I don't understand what you mean about "too similar" - we have difference in logic for both cases. Maybe you mean that new action is combination of old, but it's wide practice: for example, we have "Copy" (CTRL+C) and "Cut" (CTRL+X) actions in any editor. And it't trivial that "Cut" can be replaced by "Copy"+"Del", but we have separable action for such common operation for usability.

antonanikin planned changes to this revision.Dec 17 2016, 2:25 PM

Implement new functionality inside current 'Focus Editor' action

antonanikin retitled this revision from Add new action "Focus Editor and Hide Docks" to Add functionality for hiding unlocked docks during editor focusing.Jan 25 2017, 9:04 AM
antonanikin updated this object.
antonanikin updated this object.Jan 25 2017, 9:07 AM
apol added inline comments.Jan 25 2017, 10:43 AM
sublime/mainwindow_p.cpp
280 ↗(On Diff #10519)

I think we didn't understand each other.

It's MainWindowPrivate::toggleDocksShown that should be adapted to understand dock locking.

antonanikin updated this revision to Diff 10571.EditedJan 26 2017, 1:48 AM
  • Adapt IdealController::toggleDocksShown() methods to understand dock locking
antonanikin marked an inline comment as done.Jan 26 2017, 1:49 AM
antonanikin added inline comments.
sublime/mainwindow_p.cpp
280 ↗(On Diff #10519)

Done. Thanks for your idea about "right place" for the new code.

antonanikin marked an inline comment as done.Jan 26 2017, 1:50 AM
antonanikin updated this object.Jan 26 2017, 3:43 PM
apol added inline comments.Jan 27 2017, 12:08 AM
sublime/idealcontroller.h
73 ↗(On Diff #10571)

When do we ever hide the locked?
If a toolview is locked, it should never be hidden. Maybe removed, but not hidden.

sublime/mainwindow_p.cpp
280 ↗(On Diff #10519)

No, I'm not sure what's going on, but still we are talking different things.

  • ::focusEditor is for when you want to Focus the Editor. But it's not supposed to touch the toolviews.
  • ::toggleDocksShown should respect the Docks to be Shown, additionally to focus the editor when we are hiding the docks.
  • Revert UI settings
  • Revert MainWindowPrivate::focusEditor()

Now we only have docks locking feature and adapted IdealController::toggleDocksShown.

@apol, the revision description is not fixed now. It will be done later, when we come to a consensus :)

apol accepted this revision.Jan 27 2017, 11:15 AM
apol added a reviewer: apol.

Now I like this much better. I think it's much clearer.

Also the patch is much smaller, which is a good sign.

Thanks for putting up with me @antonanikin. Fix the commit message and let it in!

This revision is now accepted and ready to land.Jan 27 2017, 11:15 AM
antonanikin retitled this revision from Add functionality for hiding unlocked docks during editor focusing to Add functionality for docks locking.Jan 28 2017, 7:02 AM
antonanikin updated this object.
antonanikin edited edge metadata.
This revision was automatically updated to reflect the committed changes.

@apol, thanks for your attention for this revision and your ideas about the "right way" for implementing the patch goals. The final version is quite different from the start :)

apol added a comment.Jan 29 2017, 10:09 PM

@apol, thanks for your attention for this revision and your ideas about the "right way" for implementing the patch goals. The final version is quite different from the start :)

No worries, that's why we are here! :) Thanks for the patch in the first place.