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

2010-11-04 Thread Aaron Seigo 

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

Ship it!


one small change up, and then i guess it's good to go.


Screenshot: Updated

should also be disabled when the widgets are

- Aaron


On 2010-11-04 17:57:08, Alex Merry wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5747/
> ---
> 
> (Updated 2010-11-04 17:57:08)
> 
> 
> 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 
> 1192604 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.cpp 
> 1192604 
>   
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clockConfig.ui
>  1192604 
> 
> 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/
> Updated
>   http://svn.reviewboard.kde.org/r/5747/s/549/
> 
> 
> Thanks,
> 
> Alex
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

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

2010-11-04 Thread Alex Merry 

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

(Updated 2010-11-04 17:57:08.651004)


Review request for Plasma.


Changes
---

Configuration dialog updated as per Aaron's suggestions.  No changes to 
rendering code.

I'm also not happy with option proliferation, but this at least is in line with 
the other font/styling options that are already in the dialog.


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 (updated)
-

  /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.h 
1192604 
  /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.cpp 
1192604 
  
/trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clockConfig.ui
 1192604 

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/
Updated
  http://svn.reviewboard.kde.org/r/5747/s/549/


Thanks,

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

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

2010-11-04 Thread 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

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

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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

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

2010-11-04 Thread Sebastian Kügler 

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

Ship it!


Patch looks good and fixes a couple of problems with the newly introduced 
shadow.

I'm not 100% sure about the new config option, but I also don't see another way 
to get it right without it.

- Sebastian


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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

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

2010-11-03 Thread Alex Merry 

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

(Updated 2010-11-04 02:07:24.050751)


Review request for Plasma.


Changes
---

Screenshot included.

I made disabling the shadow an option because that's the only way I can get 
anything that looks half decent on my desktop (I have the default panel at the 
top of the screen with Fresh Morning as my background - the panel is 
translucent, so the theme's default colours look really bad and are hard to 
read, but plain solid white looks fine.

The reason this is a combined feature/bug fix is because I couldn't figure out 
a sensible way to fix the bug without including the option to change the shadow 
(given that black text with a black shadow, for example, is not a sensible 
combination).  Well, other than removing the option to set the text colour 
altogether, but that would give me an ugly, hard-to-read clock.


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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

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

2010-11-03 Thread Aaron Seigo 

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


can you include a screenshot of the changed config? i also wonder about the 
value of making the shadow optional (other than "because we can"). 

p.s. it's a little unfortunate that this review mixes both a bug fix and a new 
feature.

- Aaron


On 2010-11-01 18:42:29, Alex Merry wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5747/
> ---
> 
> (Updated 2010-11-01 18:42:29)
> 
> 
> 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.
> 
> 
> Thanks,
> 
> Alex
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

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

2010-11-01 Thread Alex Merry 

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

(Updated 2010-11-01 18:42:29.964037)


Review request for Plasma.


Changes
---

Added the option to disable the shadow (not all colour choices look right with 
a shadow: specifically, on my desktop with a translucent panel, it looks bad 
with anything except white text and no shadow).

Note that all translucency of text is now disabled if the shadow is not drawn.


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 (updated)
-

  /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.


Thanks,

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

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

2010-11-01 Thread Alex Merry 

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

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.


Thanks,

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel