fix #287528, fix #283258 & fix #289773: panels not dockable after undock & close #5086

Obliquely · 2019-05-31T07:29:51Z

Adds a function that tests if widget is both not-shown (!isVisible) AND
in the floating state. If so, it cycles the floating state through off
and on again to ensure widget is re-dockable. The !isVisible() test
avoids side effect of dock being resized, e.g. made wider, even when
the widget won't be landing on it (#289773). Function used to the fix
issue for:

Play Panel
Palettes
Inspector
Selection Filter
Piano Keyboard (when dockable, only dockable in footer)
Time Line (when dockable, only dockable in footer)
Score Comparison Tool
Mixer

Addresses some issues with the mixer (#283258) being converted from
an independent window to a dock widget:

add code to ensure the show/hide menu is in sync with the mixer being
shown
removed inappropriate widget configuration on setup

RobFog · 2019-06-01T23:50:11Z

Is this pull request complete and ready for review?

RobFog · 2019-06-01T23:53:52Z

For reference, here are the three issues which are meant to be fixed by this pull request:

Obliquely · 2019-06-02T05:00:55Z

Yes. Complete and ready for review.

dmitrio95

Overall, this solution looks good to me, as far as I understand it shouldn't cause any unwanted side effects as it was the case for #5011. I left some comments pointing to code style issues, but once they are corrected I believe this PR should be good to merge.

dmitrio95 · 2019-06-15T09:01:27Z

mscore/mixer.cpp

@@ -200,6 +200,10 @@ void Mixer::masterVolumeChanged(double decibels)

 void Mixer::on_partOnlyCheckBox_toggled(bool checked)
      {
+
+      if (!_activeScore) // cope with case of no score being present


This and the following if statements can probably be combined:

if (!_activeScore || !_activeScore->excerpt() return;

dmitrio95 · 2019-06-15T09:02:14Z

mscore/musescore.cpp

+//---------------------------------------------------------
+
+void MuseScore::reDisplayDockWidget(QDockWidget* widget, bool visible)
+{


Wrong indentation: we use 6-space indent for these braces (see other functions definitions)

dmitrio95 · 2019-06-15T09:03:29Z

mscore/musescore.cpp

+            // Ensure the widget is re-dockable if it has been closed and re-opened when un-docked
+            widget->setFloating(false);
+            widget->setFloating(true);
+      }


Wrong indentation: this brace should be indented one level (6 spaces) more (see the Handbook).

dmitrio95 · 2019-06-15T09:05:53Z

mscore/musescore.cpp

@@ -4424,6 +4439,10 @@ void MuseScore::readSettings()

      MuseScore::restoreGeometry(this);

+      // grab the visible state before the beginGroup
+      // this awkwardness to ensure state saved before code changes should


Could you please add a brief description of which code changes are mentioned in this comment? It will be possible to find this out via git blame but it will be even better if one could understand this comment without searching through the changes history.

dmitrio95 · 2019-06-15T09:09:00Z

mscore/musescore.cpp

+void MuseScore::reDisplayDockWidget(QDockWidget* widget, bool visible)
+{
+      if (widget->isFloating() && !widget->isVisible()) {
+            // Ensure the widget is re-dockable if it has been closed and re-opened when un-docked


Also could you please add a reference to QTBUG-69922 (something short, like see QTBUG-69922, would be enough)? Even though this is a really good workaround it is still a workaround, and it is better not to keep it in the code if that issue gets fixed (and ensure that we keep it while this Qt issue is still present).

Obliquely · 2019-06-15T14:25:16Z

Thanks for review. Requested changes made and pushed.

dmitrio95 · 2019-06-15T20:39:54Z

mscore/mixer.cpp

+//---------------------------------------------------------
+
+void Mixer::hideEvent(QHideEvent* e)
+{


6 space indent is needed too, I seem to have missed that in my last review.

Sorry I missed that too. Fixed and pushed.

Jojo-Schmitz · 2019-06-16T08:11:29Z

getting super picky: typo in commit and PR title, "afer" vs "after"
Also in order to auto close issues the commit title needs to be fix #287528, fix #283258 & fix #289773: …

…ock & close 1) Adds a function that tests if widget is both not-shown (!isVisible) AND in the floating state. If so, it cycles the floating state through off and on again to ensure widget is re-dockable. This is a workaround for QT-69922. The !isVisible() test avoids side effect of dock being resized, e.g. made wider, even when the widget won't be landing on it (#289773). Function used to the fix issue for: - Play Panel - Palettes - Inspector - Selection Filter - Piano Keyboard (when dockable, only dockable in footer) - Time Line (when dockable, only dockable in footer) - Score Comparison Tool - Mixer 2) Addresses some issues with the mixer (#283258) being converted from an independent window to a dock widget: - add code to ensure the show/hide menu is in sync with the mixer being shown - removed inappropriate widget configuration on setup

Obliquely · 2019-06-16T10:09:45Z

Thanks. Have fixed typo and changed the fix's to the appropriate format.

dmitrio95 reviewed Jun 15, 2019

View reviewed changes

Obliquely force-pushed the palette-docking-2nd-go branch from 0a2f962 to 28ad946 Compare June 15, 2019 13:14

dmitrio95 reviewed Jun 15, 2019

View reviewed changes

Obliquely force-pushed the palette-docking-2nd-go branch from 28ad946 to be28791 Compare June 16, 2019 06:59

Obliquely changed the title ~~Fixes #287528, #283258 & #289773 Panels not dockable afer undock & close~~ Fixes #287528, #283258 & #289773 Panels not dockable after undock & close Jun 16, 2019

Obliquely force-pushed the palette-docking-2nd-go branch from be28791 to e010c78 Compare June 16, 2019 09:06

Obliquely force-pushed the palette-docking-2nd-go branch from e010c78 to 86a3476 Compare June 16, 2019 09:08

Obliquely changed the title ~~Fixes #287528, #283258 & #289773 Panels not dockable after undock & close~~ fixes #287528, fix #283258 & fix #289773: panels not dockable after undock & close Jun 16, 2019

Obliquely changed the title ~~fixes #287528, fix #283258 & fix #289773: panels not dockable after undock & close~~ fix #287528, fix #283258 & fix #289773: panels not dockable after undock & close Jun 16, 2019

dmitrio95 merged commit 91ae302 into musescore:master Jun 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix #287528, fix #283258 & fix #289773: panels not dockable after undock & close #5086

fix #287528, fix #283258 & fix #289773: panels not dockable after undock & close #5086

Obliquely commented May 31, 2019

RobFog commented Jun 1, 2019

RobFog commented Jun 1, 2019

Obliquely commented Jun 2, 2019

dmitrio95 left a comment

dmitrio95 Jun 15, 2019

dmitrio95 Jun 15, 2019

dmitrio95 Jun 15, 2019

dmitrio95 Jun 15, 2019

dmitrio95 Jun 15, 2019 •

edited

Obliquely commented Jun 15, 2019

dmitrio95 Jun 15, 2019

Obliquely Jun 16, 2019

Jojo-Schmitz commented Jun 16, 2019 •

edited

Obliquely commented Jun 16, 2019

fix #287528, fix #283258 & fix #289773: panels not dockable after undock & close #5086

fix #287528, fix #283258 & fix #289773: panels not dockable after undock & close #5086

Conversation

Obliquely commented May 31, 2019

RobFog commented Jun 1, 2019

RobFog commented Jun 1, 2019

Obliquely commented Jun 2, 2019

dmitrio95 left a comment

Choose a reason for hiding this comment

dmitrio95 Jun 15, 2019

Choose a reason for hiding this comment

dmitrio95 Jun 15, 2019

Choose a reason for hiding this comment

dmitrio95 Jun 15, 2019

Choose a reason for hiding this comment

dmitrio95 Jun 15, 2019

Choose a reason for hiding this comment

dmitrio95 Jun 15, 2019 • edited

Choose a reason for hiding this comment

Obliquely commented Jun 15, 2019

dmitrio95 Jun 15, 2019

Choose a reason for hiding this comment

Obliquely Jun 16, 2019

Choose a reason for hiding this comment

Jojo-Schmitz commented Jun 16, 2019 • edited

Obliquely commented Jun 16, 2019

dmitrio95 Jun 15, 2019 •

edited

Jojo-Schmitz commented Jun 16, 2019 •

edited