Properly show/hide preview dialog
ClosedPublic

Authored by chehrlic on Aug 26 2017, 3:47 PM.

Details

Summary

The visiblity of the preview dialog was not synced with the preview action
in the main window. Also the preview image was not created when then same
puzzle was opened after going back to the overview.
Some c++11 changed (using nullptr instead 0)
Don't leak PuzzleComponent in GamePlay::loadPreview()

Test Plan
  • make sure to have a clean environment (e.g. no saved puzzles)
    • open a puzzle, activate preview
    • go back to overview
    • open another puzzle -> no preview and button state is unchecked
    • go back to overview
    • open first puzzle -> preview is open and button state is checked
    • go back to overview
    • open puzzle again -> preview does show something (instead 'Image is not available')

Diff Detail

Repository
R423 Palapeli
Branch
preview
Lint
No Linters Available
Unit
No Unit Test Coverage
chehrlic created this revision.Aug 26 2017, 3:47 PM
Restricted Application added a reviewer: KDE Games. · View Herald TranscriptAug 26 2017, 3:47 PM
Restricted Application added a subscriber: KDE Games. · View Herald Transcript

So, this patch does not reenable the preview for me, but D7566 does it! If that one is applied, is this patch still needed as it is, or just the part when the status is initial status synchronized?

This patch is still needed - D7566 was needed for the initial copying of the puzzles from the global to the local storage. This one here is needed for the sync of the preview check state button and the proper recreation of the preview image when opening the same puzzle again after switching back to overview (see test plan).

chehrlic updated this revision to Diff 18835.Aug 27 2017, 7:46 AM

don't leak preview toggleAction in MainWindow::setupActions()

ltoscano edited the summary of this revision. (Show Details)Aug 27 2017, 2:41 PM
ltoscano accepted this revision.Aug 27 2017, 2:46 PM

It works as expected. Technically the issue is not a regression, I've just noticed also in the kdelibs4 version. It *may* be fixed there, but up to you.
It was not reported as bug (btw, would you like to go through the bug list for palapeli to see if it's possible to close something?)

This revision is now accepted and ready to land.Aug 27 2017, 2:46 PM
chehrlic closed this revision.Aug 27 2017, 3:36 PM