D8289: IdealDockWidget : support detaching as regular standalone window (WIP)

[René J.V. Bertin]

rjvbb created this revision.
rjvbb added a reviewer: KDevelop.
rjvbb added a project: KDevelop.
Restricted Application added a subscriber: kdevelop-devel.

REVISION SUMMARY
  This patch explores a mechanism that allows users to detach ("float") dock widget views as regular/standalone windows instead of as Qt::Tool windows, and extends the API to make this behaviour default.
  
  The current approach achieves this by reparenting the widget of an already floated view away from its parent, as discussed on the ML. It makes sure there are no direct consequences of this (adds NULL checks for m_area and m_view where needed) and caches the original parent in a state variable so that the window can be reattached.
  
  By default, the "regular detach window" option is provided as an additional option in the "Toolview position" menu. (See test plan.)
  
  Detached standalone windows are reattached and docked on application shutdown as a precaution and to prevent them from being restored as Qt::Tool windows when the application is restarted (as far as I can tell we cannot prevent that or even rectify it post-hoc). See also test plan.
  
  An API is provided allowing clients to indicate that views should be floated as standalone windows: a private state variable with getter and setter, plus an overload for QDockWidget::setFloating(). Setting this option removes that "Detached as floating window" menu action and causes "tearing off" to give the desired window type. This API extension requires exporting IdealDockWidget.
  This might be proposed for upstreaming: support for regular detached windows is a lot easier to implement in QDockWidget (cf. https://bugreports.qt.io/browse/QTBUG-63774). Alternatively upstreaming could also target KWidgetsAddons (KDockWidget).
  
  Reparenting the detached view means the dock widget itself is left empty, which is why the patch closes it. Reparenting also provokes a change in window position. It doesn't really bother me when using the "detach as standalone" menu action but it's a bit surprising when it happens while "tearing off" a view. Ideas welcome how to prevent this.
  
  The class is now tested in the test_viewactivation unittest, which requires exporting the IdealController class as well. The test is currently very basic, and cannot yet test standalone floating mode properly because IdealController::raiseView crashes (m_view_to_action.value(view) returns NULL). The test succeeds when I remove the `m_area->raiseToolView(m_view)` call from IdealDockWidget::makeStandaloneWindow().

TEST PLAN
  Developed and tested on the 5.2 branch, on Mac and Linux.
  
  I still need to determine if it's possible to go back directly from a standalone floating window to a stock floating window, or if it's safer to disable the "Detached as floating window" action when standalone mode is active.
  
  The test_viewactivation test should be fixed so that the IdealController has a view with a valid action (for a kill? :)).
  
  Tighter integration could be nice, allowing views that were floating at shutdown to be restored as such, including the window type. I have not been able to figure out if this is at all possible or even how dock widgets are currently being restored in floating mode. Apparently Qt decides and doesn't give any feedback about it.
  As a null option, I'd prefer to re-dock views systematically at shutdown, regardless of the kind of window used for floating views.

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D8289

AFFECTED FILES
  kdevplatform/sublime/idealcontroller.h
  kdevplatform/sublime/idealdockwidget.cpp
  kdevplatform/sublime/idealdockwidget.h
  kdevplatform/sublime/tests/test_viewactivation.cpp
  kdevplatform/sublime/tests/test_viewactivation.h

To: rjvbb, #kdevelop
Cc: kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171014/baca3709/attachment.html>