-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix dock pane layout #545
Fix dock pane layout #545
Conversation
This PR definitely fixes the issue unlike #535 BUT understandably, this breaks behavior on pyqt 4 🤣 |
# Remove the fixed sizes once Qt activates the layout. | ||
QtCore.QTimer.singleShot(0, self._reset_fixed_sizes) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you undo the removals and add if/else clauses to not change the behavior on qt4? like in #535 . (i haven't checked the test suite but i'd wager reverting the changes would fix the test suite)
Not sure how to write a regression test for this though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i restored the removed code and moved into a qt4 conditional branch. but i'm still not sure how to add a regression test for this.
@pedrorivotti will you be able to work on this PR this week? If not, I can take over and update the PR (unless someone on the ETS team can take over @corranwebster ). It'd be nice to get this merged and shipped in a bug fix soon. |
@rahulporuri Please feel free to take over, I don't think I would get around to it before next week. |
modified: pyface/ui/qt4/tasks/main_window_layout.py
@kitchoi @corranwebster any guidance on adding a regression test for this? I have tested this locally on our application with qt4 and qt5 - and dock pane restoration works as expected on both toolkits. |
Codecov Report
@@ Coverage Diff @@
## master #545 +/- ##
==========================================
+ Coverage 36.99% 37.20% +0.21%
==========================================
Files 470 470
Lines 26027 26096 +69
Branches 3961 3971 +10
==========================================
+ Hits 9629 9710 +81
+ Misses 15977 15970 -7
+ Partials 421 416 -5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @corranwebster Do you want to have a look too before this goes in?
Confirmed with @corranwebster offline that this can be merged. |
@kitchoi are there plans of a bugfix release? |
I just opened a bug release check list issue for TraitsUI, and will open one for Pyface. It is not really a complete plan as in there isn't a target release date yet. Open for discussion! |
* Provide sizeHint to QWidget * REF : Restore qt4 conditional branch modified: pyface/ui/qt4/tasks/main_window_layout.py * TST: Test Qt dock widget has the correct size (#552) Co-authored-by: Sai Rahul Poruri <[email protected]> Co-authored-by: Kit Choi <[email protected]>
* Fix errors from incorrect QImage memory management. (#546) This should stop the sporadic test failures caused by referencing memory which has been freed and used by something else. The fix is to attach the numpy array with the allocated memory to the QImage, so that the lifetime of the array is at least as long as that of the QImage. * Fix dock pane layout (#545) * Provide sizeHint to QWidget * REF : Restore qt4 conditional branch modified: pyface/ui/qt4/tasks/main_window_layout.py * TST: Test Qt dock widget has the correct size (#552) Co-authored-by: Sai Rahul Poruri <[email protected]> Co-authored-by: Kit Choi <[email protected]> * Update Changelog for 7.0.1 * Remove master only setting on Travis and Appveyor which prevent bug fix release PR from building * Update .travis.yml for Qt (#529) * Update .travis.yml to include libglu1-mesa-dev for Qt * Restore libxkbcommon-x11 and move other dependencies up * Fix package name for libsdl * Add a debug flag * Add libxcb-iccm4 * Continue the same exercise for pyside2, add libxcb-image * Giving up - let's use the same packages * Add libxml2 for osx image * Use brew manually See https://travis-ci.community/t/macos-build-fails-because-of-homebrew-bundle-unknown-command/7296/28 * Try a newer osx image * Add a comment * Is libdbus still needed? * Remove debugging flag - it is noisy and is embedded in between tests * Update install command for installing wx to match master (this is equivalent to a merge of a couple of PRs Co-authored-by: Corran Webster <[email protected]> Co-authored-by: Pedro Rivotti <[email protected]> Co-authored-by: Sai Rahul Poruri <[email protected]>
This PR is an alternative to #535 that also closes #442
@rahulporuri Could you please check if this approach works for your use case?