[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:       Johannes Jowereit <jowereit () googlemail ! com>
Date:       2007-02-24 15:24:00
Message-ID: 200702241624.00369.jowereit () googlemail ! com
[Download RAW message or body]

Am Donnerstag, 22. Februar 2007 10:43 schrieben Sie:
> On Thursday 22 February 2007 01:24, Matt Rogers wrote:
> > On Wednesday 21 February 2007 13:27, Johannes Jowereit wrote:
> > > I would be glad to hear from you what you think about this
> > > mockup.
> >
> > Do you have code? I want to see this committed.
>
> I fixed the spacing issues (too much, uneven) around the edges of
> the dialog a week or two ago. For the rest I agree with Matt,
> patches are *most* welcome, the current History is uninviting at
> best.

Hello,

first of all, thanks for your encouraging answers. I have now written 
a patch (applies to Kopete version 0.12.3) that solves at least some 
of the issues I wrote about:

- The search line above the date list view has been removed.
- The status label has been removed and the progress bar is now placed 
at the bottom-left corner of the date list view (like in the file 
open dialog), where it doesn't take much space.
- The messages now have a header, even if it is not very pretty at the 
moment.
- The i18n issues have been fixed; all dates are now localized.
- The search button is removed and has been replaced by a timer which 
starts the search after a typing pause of 500ms.

Additionally, I have fixed two bugs I found during testing:
- The search now only begins when all messages have been loaded. 
Before, the progress bar did some crazy things when searching while 
the messages were loaded.
- If you change the contact with the contact combo box while messages 
are loaded, the date list view will no longer be polluted with the 
messages of the contact selected before.

In the UI, the combo boxes and the search box can now be accessed via 
keyboard shortcuts (accelerators).

The message is now not only displayed in the message viewer when the 
corresponding item is clicked, but also when it is selected e.g. by 
keyboard (it now reacts simply on the selectionChanged signal).

Several optical issues have been fixed, too.

As it is my first KDE patch, there are probably several things done 
wrong ;-) I would be glad if you could tell me, so I can do them 
better the next time.

There are also some problems I couldn't solve. Maybe you can help me 
with it:
- I commented out the code in line 374-381 (non-patched version) and 
replaced it by putting the account in the header. However, the 
original code does some checks I don't really understand. Is my code 
sufficent?
- I couldn't get Qt to let the size of the date list view fixed, so 
now it is too big and resizes when the window is resized. How can this 
be fixed?
- I wanted to include a link to chat with the person whose messages 
are displayed. In KMail, the link had a form like "im:<cryptical 
string>", but I couldn't figure out how to get that in Kopete. Or 
does this only work for contacts that are listed in the address book?
- How are chats with more than one person handled? (I don't have an 
account on which testing this is possible.) I don't think any special 
code for this was included in the original version of the history 
plugin, so I assume I don't have to care about it. Or is this a 
missing feature?

I hope my patch helps you to improve the history plugin. In any case, 
it is great contributing something to KDE. Please help me a bit by 
answering my questions.

Best regards,
Johannes

-- 
Black holes are where God divided by zero -- Steven Wright

["historydialog.cpp.diff" (text/x-diff)]

--- kopete-0.12.2-new/kopete/plugins/history/historydialog.cpp	2006-08-12 \
                02:50:49.000000000 +0200
+++ kopete-0.12.2/kopete/plugins/history/historydialog.cpp	2007-02-24 \
16:20:20.000000000 +0100 @@ -74,7 +74,7 @@
 
 
 KListViewDateItem::KListViewDateItem(KListView* parent, QDate date, \
                Kopete::MetaContact *mc)
-		: KListViewItem(parent, date.toString(Qt::ISODate), mc->displayName())
+		: KListViewItem(parent, date.toString(Qt::LocalDate), mc->displayName())
 {
 	mDate = date;
 	mMetaContact = mc;
@@ -120,7 +120,10 @@
 	mMainWidget->searchLine->setTrapReturnKey(true);
 	mMainWidget->searchErase->setPixmap(BarIcon("locationbar_erase"));
 
-	mMainWidget->contactComboBox->insertItem(i18n("All"));
+	progress = new KProgress(mMainWidget->dateListView, "progress");
+	typeTimer = new QTimer( this );
+
+	mMainWidget->contactComboBox->insertItem(i18n("All contacts"));
 	mMetaContactList = Kopete::ContactList::self()->metaContacts();
 	QPtrListIterator<Kopete::MetaContact> it(mMetaContactList);
 	for(; it.current(); ++it)
@@ -131,7 +134,6 @@
 	if (mMetaContact)
 		mMainWidget->contactComboBox->setCurrentItem(mMetaContactList.find(mMetaContact)+1);
  
-	mMainWidget->dateSearchLine->setListView(mMainWidget->dateListView);
 	mMainWidget->dateListView->setSorting(0, 0); //newest-first
 
 	setMainWidget(mMainWidget);
@@ -156,7 +158,7 @@
 	l->addWidget(mHtmlView);
 
 	QTextOStream( &fontSize ) << KopetePrefs::prefs()->fontFace().pointSize();
-	fontStyle = "<style>.hf { font-size:" + fontSize + ".0pt; font-family:" + \
KopetePrefs::prefs()->fontFace().family() + "; color: " + \
KopetePrefs::prefs()->textColor().name() + "; }</style>"; +	fontStyle = "<style>.hf { \
font-size:" + fontSize + ".0pt; font-family:" + \
KopetePrefs::prefs()->fontFace().family() + "; color: " + \
KopetePrefs::prefs()->textColor().name() + "; } .header { border:1px solid black; \
margin-bottom: 5px } .conv-heading { border-bottom: 1px solid black; \
background-color: #447bcd; padding: 5px; font-weight: bold; color: white } \
.conv-details { background: #EEEEEE; padding: 5px } </style>";  
 	mHtmlPart->begin();
 	htmlCode = "<html><head>" + fontStyle + "</head><body class=\"hf\"></body></html>";
@@ -166,14 +168,15 @@
 	
 	connect(mHtmlPart->browserExtension(), SIGNAL(openURLRequestDelayed(const KURL &, \
const KParts::URLArgs &)),  this, SLOT(slotOpenURLRequest(const KURL &, const \
                KParts::URLArgs &)));
-	connect(mMainWidget->dateListView, SIGNAL(clicked(QListViewItem*)), this, \
                SLOT(dateSelected(QListViewItem*)));
-	connect(mMainWidget->searchButton, SIGNAL(clicked()), this, SLOT(slotSearch()));
+	connect(mMainWidget->dateListView, SIGNAL(selectionChanged(QListViewItem*)), this, \
SLOT(dateSelected(QListViewItem*)));  connect(mMainWidget->searchLine, \
SIGNAL(returnPressed()), this, SLOT(slotSearch()));  connect(mMainWidget->searchLine, \
SIGNAL(textChanged(const QString&)), this, SLOT(slotSearchTextChanged(const \
QString&)));  connect(mMainWidget->searchErase, SIGNAL(clicked()), this, \
SLOT(slotSearchErase()));  connect(mMainWidget->contactComboBox, \
SIGNAL(activated(int)), this, SLOT(slotContactChanged(int)));  \
connect(mMainWidget->messageFilterBox, SIGNAL(activated(int)), this, \
SLOT(slotFilterChanged(int )));  connect(mHtmlPart, SIGNAL(popupMenu(const QString &, \
const QPoint &)), this, SLOT(slotRightClick(const QString &, const QPoint &))); \
+	connect(mMainWidget, SIGNAL(resize(const QSize &)), this, SLOT(slotResize(const \
QSize &))); +	connect(typeTimer, SIGNAL(timeout()), this, SLOT(slotSearch())); // \
search after a typing pause  
 	//initActions
 	KActionCollection* ac = new KActionCollection(this);
@@ -197,14 +200,20 @@
 
 void HistoryDialog::init()
 {
+	// if there is another init running, clear it (prevents unnecessary delays
+	// at the start of the search)
+	mInit.dateMCList.clear();
+
 	if(mMetaContact)
 	{
 		delete mLogger;
 		mLogger= new HistoryLogger(mMetaContact, this);
+		mInit.mc = mMetaContact;
 		init(mMetaContact);
 	}
 	else
 	{
+		mInit.mc = 0L;
 		QPtrListIterator<Kopete::MetaContact> it(mMetaContactList);
 		for(; it.current(); ++it)
 		{
@@ -216,7 +225,8 @@
 
 	}
 
-	initProgressBar(i18n("Loading..."),mInit.dateMCList.count());
+	initProgressBar(mInit.dateMCList.count());
+
 	QTimer::singleShot(0,this,SLOT(slotLoadDays()));
 }
 
@@ -231,8 +241,17 @@
 		}
 		
 		DMPair pair(mInit.dateMCList.first());
+
+		// FIXME: Does this ever occur?
+		if (((!mMetaContact) and (mInit.mc)) ||
+		     ((mInit.mc) && (QString::compare(mMetaContact->metaContactId(), \
mInit.mc->metaContactId())))) { +				doneProgressBar();
+				return;
+		}
+
 		mInit.dateMCList.pop_front();
 		mLogger= new HistoryLogger(pair.metaContact(), this);
+
 		QValueList<int> dayList = mLogger->getDaysForMonth(pair.date());
 		for (unsigned int i=0; i<dayList.count(); i++)
 		{
@@ -242,7 +261,7 @@
 		}
 		delete mLogger;
 		mLogger = 0;
-		mMainWidget->searchProgress->advance(1);
+		progress->advance(1);
 		QTimer::singleShot(0,this,SLOT(slotLoadDays()));
 
 
@@ -338,10 +357,10 @@
 	delete mLogger;
 	mLogger = 0;
 
-	setMessages(msgs);
+	setMessages(msgs, item->metaContact());
 }
 
-void HistoryDialog::setMessages(QValueList<Kopete::Message> msgs)
+void HistoryDialog::setMessages(QValueList<Kopete::Message> msgs, \
Kopete::MetaContact *mc)  {
 	// Clear View
 	DOM::HTMLElement htmlBody = mHtmlPart->htmlDocument().body();
@@ -353,10 +372,57 @@
 		QString::fromLatin1("ltr"));
 
 	QValueList<Kopete::Message>::iterator it = msgs.begin();
+	QString recipientsHTML;
+	QString iconPath = KGlobal::iconLoader()->iconPath( \
(*it).from()->protocol()->pluginIcon(), KIcon::Small ); +	QString iconHTML = \
(iconPath != "") ? QString("<img src=\"%1\" width=\"16\" height=\"16\" \
style=\"margin-right: .3em\"/>").arg(iconPath) : ""; +	// FIXME: Are the recipients \
really stored in mc->contacts() if there are multiple recipients? \
+	QPtrList<Kopete::Contact> recipients = mc->contacts(); +
+	if (recipients.count() == 1)
+		recipientsHTML = mc->displayName();
+	else {
+		QPtrListIterator<Kopete::Contact> cit( recipients );
+	
+		recipientsHTML  = "<ul>";
+		for( ; cit.current(); ++cit )
+		{
+			recipientsHTML += "<li></li>";
+			
+		}
+		recipientsHTML += "</ul>";
+	}
 
+	// taken from KMail headerstyle.cpp
+	QString dateString;
+	QDateTime dateTime;
+	KLocale* locale = KGlobal::locale();
+	dateTime = ( (*it).timestamp() );
+	dateString = locale->formatDate( dateTime.date(), false );
+
+	QString resultHTML = "<div class=\"header\" dir=\"" + dir + "\"><div \
class=\"conv-heading\">" +		+ i18n("Conversation with %1").arg(recipientsHTML)
+		+ "</div>"
+		+ "<div class=\"conv-details\">"
+		+ "<table>"
+		+ "<tr><td style=\"font-weight: bold\">" + i18n("Me:") + "</td><td>" + iconHTML + \
(*it).from()->account()->accountLabel() + "</td></tr>" + 		+ "<tr><th \
style=\"font-weight: bold\">" + i18n("Conversation with:") + "</td>"; +
+	QPtrListIterator<Kopete::Contact> cit( recipients );
+	recipientsHTML = "";
+
+	for( ; cit.current(); ++cit )
+	{
+		// FIXME: Does this have to be i18n'ed?
+		recipientsHTML += QString("<td>%3%1 \
(%2)</td>").arg((*cit)->contactId()).arg((*cit)->nickName()).arg(iconHTML); +		if \
(!cit.atLast()) +			recipientsHTML += "</tr><tr><td></td>";
+	}
+
+	resultHTML += recipientsHTML + "</tr>"
+		+ "<tr><td><b>" + i18n("Date:") + "</b></td><td>" + dateString + "</td></tr>"
+		+ "</table>"
+		+ "</div></div>";
 
-	QString accountLabel;
-	QString resultHTML = "<b><font color=\"red\">" + \
(*it).timestamp().date().toString() + "</font></b><br/>";  DOM::HTMLElement newNode = \
mHtmlPart->document().createElement(QString::fromLatin1("span"));  \
newNode.setAttribute(QString::fromLatin1("dir"), dir);  \
newNode.setInnerHTML(resultHTML); @@ -371,14 +437,14 @@
 		{
 			resultHTML = "";
 	
-			if (accountLabel.isEmpty() || accountLabel != \
                (*it).from()->account()->accountLabel())
-			// If the message's account is new, just specify it to the user
-			{
-				if (!accountLabel.isEmpty())
-					resultHTML += "<br/><br/><br/>";
-				resultHTML += "<b><font color=\"blue\">" + \
                (*it).from()->account()->accountLabel() + "</font></b><br/>";
-			}
-			accountLabel = (*it).from()->account()->accountLabel();
+// 			if (accountLabel.isEmpty() || accountLabel != \
(*it).from()->account()->accountLabel()) +// 			// If the message's account is new, \
just specify it to the user +// 			{
+// 				if (!accountLabel.isEmpty())
+// 					resultHTML += "<br/><br/><br/>";
+// 				resultHTML += "<b><font color=\"blue\">" + \
(*it).from()->account()->accountLabel() + "</font></b><br/>"; +// 			}
+// 			accountLabel = (*it).from()->account()->accountLabel();
 	
 			QString body = (*it).parsedBody();
 	
@@ -388,7 +454,7 @@
 				body = body.replace(mMainWidget->searchLine->text(), "<span \
style=\"background-color:yellow\">" + mMainWidget->searchLine->text() + "</span>", \
false);  }
 		
-			resultHTML += "(<b>" + (*it).timestamp().time().toString() + "</b>) "
+			resultHTML += "(" + (*it).timestamp().time().toString() + ") "
 					+ ((*it).direction() == Kopete::Message::Outbound ?
 									"<font color=\"" + KopetePrefs::prefs()->textColor().dark().name() + \
                "\"><b>&gt;</b></font> "
 									: "<font color=\"" + KopetePrefs::prefs()->textColor().light(200).name() + \
"\"><b>&lt;</b></font> ") @@ -414,17 +480,20 @@
 	new KRun(url, 0, false); // false = non-local files
 }
 
-// Disable search button if there is no search text
+// Disable search button if there is no search text and
+// re-start the typing pause timer.
 void HistoryDialog::slotSearchTextChanged(const QString& searchText)
 {
 	if (searchText.isEmpty())
 	{
-		mMainWidget->searchButton->setEnabled(false);
-		slotSearchErase();
+		typeTimer->stop();
+		slotSearchErase();		
 	}
 	else
 	{
-		mMainWidget->searchButton->setEnabled(true);
+		typeTimer->stop();
+		if(mInit.dateMCList.isEmpty()) /* only search when contact list is loaded */
+			typeTimer->start(500, true);
 	}
 }
 
@@ -433,17 +502,24 @@
 	KListViewDateItem* item = \
static_cast<KListViewDateItem*>(mMainWidget->dateListView->firstChild());  while \
(item != 0)  {
-			item->setVisible(s);
-			item = static_cast<KListViewDateItem*>(item->nextSibling());
+		item->setVisible(s);
+		item = static_cast<KListViewDateItem*>(item->nextSibling());
 	}
 }
 
-// Erase the search line, show all date/metacontacts items in the list (accordint to \
                the
-// metacontact selected in the combobox)
+// Erase the search line, show all date/metacontacts items in the list (according to \
the +// metacontact selected in the combobox) and stop a running search
 void HistoryDialog::slotSearchErase()
 {
 	mMainWidget->searchLine->clear();
 	listViewShowElements(true);
+	if (mSearch)
+	{
+		delete mSearch;
+		mSearch = 0L;
+		doneProgressBar();
+		return;
+	}
 }
 
 // Search initialization
@@ -467,7 +543,6 @@
 
 	if (mSearch)
 	{
-		mMainWidget->searchButton->setText(i18n("&Search"));
 		delete mSearch;
 		mSearch = 0L;
 		doneProgressBar();
@@ -482,8 +557,7 @@
 	mSearch->item = 0;
 	mSearch->foundPrevious = false;
 
-	initProgressBar(i18n("Searching..."), mMainWidget->dateListView->childCount() );
-	mMainWidget->searchButton->setText(i18n("&Cancel"));
+	initProgressBar(mMainWidget->dateListView->childCount() );
 
 	mSearch->item = static_cast<KListViewDateItem*>(mMainWidget->dateListView->firstChild());
  searchFirstStep();
@@ -545,7 +619,7 @@
 	if(mSearch->item != 0)
 	{
 		// Next iteration
-		mMainWidget->searchProgress->advance(1);
+		progress->advance(1);
 
 		QTimer::singleShot(0,this,SLOT(searchFirstStep()));
 	}
@@ -558,7 +632,6 @@
 				mSearch->item->setVisible(true);
 		}
 		while(mSearch->item = static_cast<KListViewDateItem \
                *>(mSearch->item->nextSibling()));
-		mMainWidget->searchButton->setText(i18n("&Search"));
 
 		delete mSearch;
 		mSearch = 0L;
@@ -566,38 +639,37 @@
 	}
 }
 
-
-
 // When a contact is selected in the combobox. Item 0 is All contacts.
 void HistoryDialog::slotContactChanged(int index)
 {
 	mMainWidget->dateListView->clear();
 	if (index == 0)
 	{
-        setCaption(i18n("History for All Contacts"));
-        mMetaContact = 0;
+        	setCaption(i18n("History for All Contacts"));
+        	mMetaContact = 0;
 		init();
 	}
 	else
 	{
 		mMetaContact = mMetaContactList.at(index-1);
-        setCaption(i18n("History for %1").arg(mMetaContact->displayName()));
+        	setCaption(i18n("History for %1").arg(mMetaContact->displayName()));
 		init();
 	}
 }
 
-void HistoryDialog::initProgressBar(const QString& text, int nbSteps)
+void HistoryDialog::initProgressBar(int nbSteps)
 {
-		mMainWidget->searchProgress->setTotalSteps(nbSteps);
-		mMainWidget->searchProgress->setProgress(0);
-		mMainWidget->searchProgress->show();
-		mMainWidget->statusLabel->setText(text);
+	progress->adjustSize();
+	progress->move(2, mMainWidget->dateListView->height() - progress->height() -2);
+	progress->setTotalSteps(nbSteps);
+	progress->setProgress(0);
+	progress->raise();
+	progress->show();
 }
 
 void HistoryDialog::doneProgressBar()
 {
-		mMainWidget->searchProgress->hide();
-		mMainWidget->statusLabel->setText(i18n("Ready"));
+	progress->hide();
 }
 
 void HistoryDialog::slotRightClick(const QString &url, const QPoint &point)
@@ -638,4 +710,9 @@
 	connect( kapp->clipboard(), SIGNAL( selectionChanged()), mHtmlPart, \
SLOT(slotClearSelection()));  }
 
+void HistoryDialog::slotResize( const QSize & )
+{
+	progress->move(2, mMainWidget->dateListView->height() - progress->height() - 2);
+}
+
 #include "historydialog.moc"


["historydialog.h.diff" (text/x-diff)]

--- kopete-0.12.2-new/kopete/plugins/history/historydialog.h	2006-08-12 02:50:49.000000000 +0200
+++ kopete-0.12.2/kopete/plugins/history/historydialog.h	2007-02-24 07:00:40.000000000 +0100
@@ -1,4 +1,4 @@
-/*
+		/*
     kopetehistorydialog.h - Kopete History Dialog
 
     Copyright (c) 2002 by  Richard Stellingwerff <remenic@linuxfromscratch.org>
@@ -24,6 +24,7 @@
 
 #include <kdialogbase.h>
 #include <klistview.h>
+#include <kprogress.h>
 
 #include "kopetemessage.h"
 
@@ -99,11 +100,13 @@
 		void slotCopy();
 		void slotCopyURL();
 
+		void slotResize( const QSize & );
+
 	private:
 		enum Disabled { Prev=1, Next=2 };
 		void refreshEnabled( /*Disabled*/ uint disabled );
 
-		void initProgressBar(const QString& text, int nbSteps);
+		void initProgressBar(int nbSteps);
 		void doneProgressBar();
 		void init(Kopete::MetaContact *mc);
 		void init(Kopete::Contact *c);
@@ -111,7 +114,7 @@
 		/**
 		 * Show the messages in the HTML View
 		 */
-		void setMessages(QValueList<Kopete::Message> m);
+		void setMessages(QValueList<Kopete::Message> msgs, Kopete::MetaContact *mc);
 
 		void listViewShowElements(bool s);
 
@@ -129,14 +132,18 @@
 
 		QPtrList<Kopete::MetaContact> mMetaContactList;
 
+		QTimer *typeTimer;
+
 		// History View
 		KHTMLView *mHtmlView;
 		KHTMLPart *mHtmlPart;
 		HistoryViewer *mMainWidget;
 		Kopete::XSLT *mXsltParser;
+		KProgress *progress;
 
 		struct Init
 		{
+			Kopete::MetaContact *mc; // which mc's log is initialized?
 			QValueList<DMPair> dateMCList; // mc for MetaContact
 		} mInit;
 

["historyviewer.ui" (application/x-designer)]

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

Configure | About | News | Add a list | Sponsored by KoreLogic