[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-pim
Subject: Re: [Kde-pim] [PATCH] KAgenda L&F changes
From: Cornelius Schumacher <schumacher () kde ! org>
Date: 2003-07-23 9:53:28
[Download RAW message or body]
On Wednesday 23 July 2003 01:13, Tim Jansen wrote:
> On Monday 21 July 2003 02:12, Tim Jansen wrote:
> > I'd like to commit the attached patch.
> > It does the following:
> > - complete redesign of the agenda item look with custom renderer
> > http://www.tjansen.de/blog/pics/after2.png
>
> Ok to commit?
I have looked at the patch and I like it :-). This is a great improvement.
Some nitpicking and some more serious issues:
- KOAgendaItem::updateIcons(): Instead of "if(mIncidence->isReadOnly())
mIconReadonly = true; else mIconReadonly = false;" you could simply write
"mIconReadOnly = mIncidence->isReadOnly()", same for the other lines like
that. Does it make sense at all to use all this variables. Wouldn't it be
easier to access the values by mIncidence and the appropriate functions in
the painting code?
- You should put the opening brace of a function on a new line (see e.g.
conditionalPaint() )
- Please don't use tabs (some of the KWordWrap calls contain tabs).
- Why do you rename the config file group for the category colors? This means
that the user loses hist custom category color settings, right?
- All-day events shouldn't never have a time header.
- Wouldn't it look better to center the text on the item, at least when it
isn't wrapped? Especially with multiple day all-day events it looks odd when
the text is on the
- When the highlight color is very dark, the text becomes invisible when the
item is selected. The text of the item should be switched to white, if the
background color becomes to dark to provide sufficient contrast.
- When the highlight color is very similar to the color of the event (e.g.
because of a category color), it becomes impossible to tell, if an event is
selected or not. I think we need another way to indicate selection of an
event than only changing the color.
- Todos shouldn't have an end time shown.
- I'm not sure that I agree with you on the pixel spacing between items.
Without this spacing it looks better in my opinion. Vertically there is no
such spacing anyway. Without the spacing it looks like all fits perfectly
together, it reminds me of LEGO ;-), and it's still no problem to see where
one event end and another starts because of the thin line at the border of
the item.
--
Cornelius Schumacher <schumacher@kde.org>
_______________________________________________
kde-pim mailing list
kde-pim@mail.kde.org
http://mail.kde.org/mailman/listinfo/kde-pim
kde-pim home page at http://pim.kde.org/
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic