Differential D2915

Fix Hide/Restore Docks
ClosedPublic
Actions

Authored by antonanikin on Oct 3 2016, 9:11 AM.

Details

Reviewers
brauch
Group Reviewers
KDevelop
Commits
R32:daeed5f185e1: Fix Hide/Restore Docks
R33:daeed5f185e1: Fix Hide/Restore Docks
Summary

The patch fixes Hide/Restore Docks functionality. The logic of current version seems to be broken:

  1. Open some left dock element
  2. Open some bottom dock element
  3. Hide docks with shortcut
  4. Restore docks with shortcut - we see both docks, as planned.
  5. Close bottom dock element
  6. Hide docks with shortcut
  7. Restore docks with shortcut - we again see both docks, but should see only left dock.

The patched version restores only docks, which was opened before hide operation.

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 7042.Oct 3 2016, 9:11 AM
antonanikin retitled this revision from to Fix Hide/Restore 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 3 2016, 9:11 AM
brauch requested changes to this revision.Oct 3 2016, 2:05 PM
brauch added a reviewer: brauch.
brauch added a subscriber: brauch.

Didn't test, but your description fits what the code looks like and I agree that your suggested behaviour is better.

I would suggest to

  • move isBarChecked() into a member function if IdealButtonBarWidget and call it isShown or so
  • change the static bools into members of IdealButtonBarWidget, and maybe call it "wasHidden" or something similar

You could easily implement isBarChecked() using std::any_of, but if that really improves readability is up to you.

This revision now requires changes to proceed.Oct 3 2016, 2:05 PM
antonanikin updated this revision to Diff 7072.Oct 4 2016, 4:45 AM
antonanikin edited edge metadata.
apol added a subscriber: apol.Oct 4 2016, 10:59 AM

I agree the current state isn't really good, I've also found some cases where it felt a bit inconsistent.

sublime/idealcontroller.cpp
457

Shouldn't we only save the state when we're hiding?

antonanikin marked an inline comment as done.Oct 4 2016, 11:07 AM
antonanikin added inline comments.
sublime/idealcontroller.cpp
457

Current version already do it only before hiding. It any bar is shown we save them states and hides it all in next code lines.

brauch accepted this revision.Oct 4 2016, 3:32 PM
brauch edited edge metadata.

Looks good to me, thanks.

This revision is now accepted and ready to land.Oct 4 2016, 3:32 PM
Closed by commit R33:daeed5f185e1: Fix Hide/Restore Docks (authored by antonanikin). · Explain WhyOct 5 2016, 8:57 AM
This revision was automatically updated to reflect the committed changes.
antonanikin marked an inline comment as done.

Revision Contents
Changeset List

Diff 7109

View Options

sublime/idealbuttonbarwidget.h

Loading...
View Options

sublime/idealbuttonbarwidget.cpp

Loading...
View Options

sublime/idealcontroller.cpp

Loading...