Review Request: Make custom colours work on the digital clock again

[Aaron Seigo]

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


so once again we descend into a situation where we have a configuration dialog with a billion twiddly options in it. i'm really not very impressed by this, to be honest.

the layout could perhaps be a bit cleaner, e.g.:

  Custom font color: [ x ] ( Color button )
        Show shadow: [ x ]
Custom shadow color: [ x ] ( Color button )

that's probably clearer and is both one less row and one less widget with the same features.


Screenshot: The changed configuration
<http://svn.reviewboard.kde.org//r/5747/#scomment42>
    when this is unchecked, both the label and the colour combo should be disabled. it's not really clear to me that the checkbox turns off the shadow versus just turns off a custom color for the shadow.


Screenshot: The changed configuration
<http://svn.reviewboard.kde.org//r/5747/#scomment41>
    shouldn't have a colon at the end of it, and since there are two colors, it should probably be plural ("colors"), ditto for "Use theme color" above.

- Aaron


On 2010-11-04 02:07:24, Alex Merry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5747/
> -----------------------------------------------------------
> 
> (Updated 2010-11-04 02:07:24)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Since the shadow was introduced for the digital clock a few weeks ago, the custom colour setting has been ignored.  This re-enables it, and also allows the user to choose a shadow colour.
> 
> This changes the configuration dialog and introduces a new option, which is why I'm submitting it for review before committing.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.h 1191270 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.cpp 1191270 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clockConfig.ui 1191270 
> 
> Diff: http://svn.reviewboard.kde.org/r/5747/diff
> 
> 
> Testing
> -------
> 
> Changing, saving and loading the settings worked in plasmoid-viewer.
> 
> 
> Screenshots
> -----------
> 
> The changed configuration
>   http://svn.reviewboard.kde.org/r/5747/s/546/
> 
> 
> Thanks,
> 
> Alex
> 
>

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