Review Request: Fifteen puzzle config dialog upgrade

Marco Martin notmart at gmail.com
Wed Oct 13 10:32:14 CEST 2010



> On 2010-10-12 08:21:53, Marco Martin wrote:
> > I think those changes are good, i'm just not sure about the removal of the shuffle button.
> > there should be at least a context button action with it, because should be possible to reshuffle it in any moment (i.e. want to start a new game)
> 
> Anthony Bryant wrote:
>     As I said, I'm working on another patch which will add a shuffle button to the solved board. However, in the mean time there is already a context menu entry for shuffling the puzzle.
> 
> Marco Martin wrote:
>     uhm, ok.
>     i'm still not sure if shuffle while still playing is visible enough but let's try for this
> 
> Anthony Bryant wrote:
>     In that case, I'll try to rework my other patch to add a bar above/below the puzzle with a shuffle button and the current elapsed time.
> 
> Marco Martin wrote:
>     uhm, let's try to keep it this way for now, this on the other hand would add a bit of visual noise, so let's add it only in the future if soe valid complaint comes over ;)
> 
> Anthony Bryant wrote:
>     It would add to the visual noise, but at the moment when you solve the puzzle it pops up a dialog (to tell you how long you took) that covers the whole plasmoid, which might be a bit worse. If I added the bar at the bottom, I could move the time elapsed display into it and get rid of the popup.
>     Of course, the other solution is to remove the time elapsed display altogether - but that would be removing a feature... What do you think?

it's worse indeed, let's go for the status asrea so


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5608/#review8084
-----------------------------------------------------------


On 2010-10-12 02:17:13, Anthony Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5608/
> -----------------------------------------------------------
> 
> (Updated 2010-10-12 02:17:13)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch fixes some usability problems in the fifteen puzzle's config:
> * Don't apply the settings before the user presses OK. This was counter-intuitive and broke the cancel button.
> * Remove the shuffle button, it doesn't really belong in the config to begin with (I'm working on a patch to add it in the bottom right of the solved puzzle).
> * Only update the size of the puzzle if the user actually changed it (this prevents unnecessary resets to the solved view when the user eg. toggles the numerals).
> * Use configChanged() and configAccepted() properly.
> 
> Also, don't update the pixmaps unless they actually need to be updated, this fixes some inefficiencies (eg. when moving pieces).
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/CMakeLists.txt 1184493 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteen.cpp 1184493 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteenPuzzle.h 1184493 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteenPuzzle.cpp 1184493 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteenPuzzleConfig.h 1184493 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteenPuzzleConfig.cpp 1184493 
>   /trunk/KDE/kdeplasma-addons/applets/fifteenPuzzle/src/fifteenPuzzleConfig.ui 1184493 
> 
> Diff: http://svn.reviewboard.kde.org/r/5608/diff
> 
> 
> Testing
> -------
> 
> Tested changing between different configurations.
> 
> 
> Screenshots
> -----------
> 
> New config dialog
>   http://svn.reviewboard.kde.org/r/5608/s/534/
> 
> 
> Thanks,
> 
> Anthony
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20101013/2d5c7103/attachment.htm 


More information about the Plasma-devel mailing list