[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kopete-devel
Subject:    Re: [kopete-devel] Usability issues in Chat History dialog
From:       Martijn Klingens <klingens () kde ! org>
Date:       2007-03-16 9:40:11
Message-ID: 200703161040.11865.klingens () kde ! org
[Download RAW message or body]

Hi Johannes,

As you may have noticed I haven't had time to review the rest of your patches 
yet.

It may take some time before I can get to that; if all else fails I'm sure 
someone else can take over too.

But for now, what about the part that I already reviewed? What about my 
comments? How would you like to move on?

Cheers,

Martijn

On Wednesday 28 February 2007 23:41, Martijn Klingens wrote:
> I'm going to reply to only part of your mail, since I don't have time to go
> over all of it now. I'll try to follow-up on the rest later, but feel free
> to beat me if I forget :)
>
> On Wednesday 28 February 2007, Johannes Jowereit wrote:
> > > Speaking of which, do you think you'll be able to work on Kopete
> > > for KDE 4 as well? We could use more developers there...
> >
> > Yes, this would be possible, as it's maybe a good entry point for KDE
> > programming. Is there already a KDE4 version of Kopete?
>
> Yes. I myself am not working on it yet, most of the others are. If you
> checkout trunk from SVN you get the KDE 4 version (you need a KDE 4 kdelibs
> too and keep it up-to-date, follow the instructions on kde.org)
>
> To get the current 3.5.x code, checkout branches/KDE/3.5 instead from SVN.
>
> > BTW, I have subscribed kopete-devel now, so you don't have to CC me
> > any more.
>
> Welcome :)
>
> You probably want to filter bugzilla mails (containing the X-Bugzilla
> header) in a folder separate from the normal mailing list traffic btw.
>
> > It has been completely removed; as I explained in my mock-up, it is
> > not needed because you can filter for the contacts with the combo box
> > and sort by date by clicking on the corresponding list view column.
>
> I need more time to think and comment on this.
>
> > > [The message header] is a new feature and probably a no-go for KDE
> > > 3.5, especially given that it introduces new strings.
> > >
> > > For KDE 4 it's a good idea.
> >
> > OK, then I'll continue developing on it, since it is not really mature
> > yet.
>
> It makes no sense if we go the 'use chatwindow code' route though...
>
> > >   Don't do that. Using arg() [...]
> >
> > OK, thanks. BTW, I think I found that way of calling QString.arg()
> > somewhere else in KDE, so it isn't fixed everywhere :-)
>
> Right. In many places it's not a bug btw, it is only a bug if %1 or %2 will
> be replaced by strings that can potentially contain the literal %1 or %2
> themselves too. Otherwise it's just slower (but probably not noticeable in
> most places.)
>
> > > * Should we use the chatwindow style here too, to make it look
> > > consistent?
> >
> > This sounds like a good idea,especially when sharing the chatwindow's
> > code with the one of the history plugin. On the other hand, some
> > chatwindow styles take much space which might be unfit for having a
> > quick overview of what you have written. So maybe a "text mode" style
> > should be provided for the history plugin, which would look much like
> > the one used now.
>
> Having a separate style for history might be a good idea, but at least I
> would like to see the code itself shared. Now we have two places where
> basically the same stuff is done, and there are inconsistencies in some of
> it, also e.g. emoticon handling and the like.
>
> > No, thanks for the tip. I guess I'll have to get familiar to SVN
> > anyway if I want to start with KDE programming ...
>
> It's a plus, also the tools from kdesdk/scripts like svnrevertlast are very
> nice. To get started you only need to know 'svn up' (update working
> copy), 'svn diff' (guess...) and 'svn commit'. You almost certainly want to
> learn 'svn status', 'svn log' and 'svn annotate' next.
>
> > > QSizePolicy::Fixed? But I'm not sure if hardcoding is a good idea.
> > > A splitter might be better.
> >
> > Yes, I wanted to keep the splitter, but when resizing the window, only
> > the message viewer should resize (as it is done now, but I changed
> > the layouts and it doesn't work any more). But I guess a little more
> > playing around with QSizePolicy should help.
>
> I need more time to comment on this.
>
> > > The cryptical string is the ID from the address book. But within
> > > Kopete you don't need to make this indirection. I think the chat
> > > window already does this. Take a look at that. But this really
> > > screams for sharing the code between history and chat window.
> >
> > Yes, indeed - maybe for KDE4 it would be possible to put the chat
> > window code into a separate class.
>
> See above :)
>
> > > It's *badly* missing. Kopete makes a mess of history of
> > > multi-person chats.
> > >
> > > I have some chat logs with 3-person chats, but when viewing them in
> > > History I don't see who wrote what, history assumes it's all
> > > written by one person :S
> > >
> > > The data on disk is correct, it's the display that's broken, so it
> > > can be fixed for my existing logs btw.
> >
> > I see ... Maybe I can try implementing something; this would again be
> > a thing for KDE4, I guess.
>
> Arguably it's a bug fix, but it depends a lot on how difficult and
> intrusive the change is whether it can go in 3.5. Even while it annoys me
> as it is now, I think it's probably wiser to let myself and other users
> suffer until KDE 4...
>
> > OK, see attachment. I made this big patch more as a "proof of
> > concept", but I see it's better if some parts are split for KDE 3.5.
>
> Thanks! I have reviewed a few, see below.
>
> > > Those can probably all go into KDE 3.5 right away and make the
> > > remaining patch a fair bit shorter.
> >
> > I think I'll continue working on the rest (header, UI, maybe I'll try
> > to get something done about multi-person chats) which could then
> > maybe go into KDE4.
>
> In that case, because KDE 4/Qt 4 are at some places different, please
> develop against KDE 4 directly. Patches written for 3.5 are not always
> straightforward to port.
>
> Now, on to the patches:
> > - accelerators: ddds keyboard accelerators to the dialog.
>
> This is not a diff, but a new .ui file. Not bad per se, but harder to
> review. Will get to that one later.
>
> > - bugfixes: fixes two bugs in the plugin:
>
> Haven't tested, but seems ok.
>
> > - i18n: The dates in the list view "Date" column and at the top of the
> > message view are now localized.
>
> This one is ok. Do you have an SVN account or do you want me to commit?
>
> > - searchErase: A click on the "Erase" button now cancels a running
> > search, which would otherwise run unneccesarily on in the background.
>
> Haven't tested or reviewed yet.
>
> > - selectionChanged: The message is now loaded when a item in the date
> > list view is selected, no matter if by mouse or by keyboard.
>
> Haven't tested or reviewed yet.
>
> > - typography: The time in the messages is now no longer displayed in bold
> > face (as an alternative, one could put the <b></b> tags around the
> > parentheses, too).
>
> I'm going to reject this one, as it changes looks in the stable Kopete. The
> current way of displaying is not exactly pretty, but it works. Removing the
> bold attribute makes it harder to read. Putting the parentheses between the
> <b> as you mentioned as alternative would be acceptable though but not
> worth the work if we go to the chat window styles in KDE 4 anyway. I could
> commit it like that of course if you like.
>
> Bed time :)
>
> And thanks again for the good stuff you're doing!
_______________________________________________
kopete-devel mailing list
kopete-devel@kde.org
https://mail.kde.org/mailman/listinfo/kopete-devel
[prev in list] [next in list] [prev in thread] [next in thread]