--===============1706867191248347346== Content-Type: multipart/alternative; boundary="===============2111662900550832728==" --===============2111662900550832728== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Jan. 4, 2012, 3:52 p.m., Martin Klapetek wrote: > > By looking at the screenshot, it's not really clear what is that klinee= dit and what is it actually for. I quickly scrolled through the code and I = saw no click-message either, so I assume that the text will simply appear t= here (even letter by letter)? I'd like to see some label there. Also the ic= on looks wrong. I know you said it looks wrong on the right too, but curren= tly it looks just randomly placed, it doesn't align with the horizontal edg= es/visual guidelines. How about moving the icon on the top right, making it= bigger and putting big "Jabber" next to it (maybe bold? though that's gene= raly bad) and another line below it saying something like "You're about to = create a jabber account". Then make the display name part of the form below= , but label it with something like "Account name". > > = > > And don't forget that we're in freeze right now ;) > = > Martin Klapetek wrote: > I meant top left, not top right, sorry :) The lineedit has a "Placeholder Text" that at the moment shows "Displayed N= ame", so that's what the user will see when he adds a new account, then yes= , the text will appear letter by letter. The placeholder also appears if th= e string is empty. Moreover if the user leaves it empty he will receive an = error. When the account is added, whatever the user chose is shown in the m= ain window. You cannot set the text "You're about to create a jabber account", because = the same widget is used both for creating and editing the account In my opinion it must be clear to the user that the display name does _not_= belong to the form below, since is a personal preference and not a paramet= er to connect to the account (that's why I used the frame). Moreover, the o= rg.freedesktop.Telepathy.Account interface supports setting the icon proper= ty, so in the future we could let the user choose the icon or do something = like kopete that lets you choose the colour of the icon, useful to distingu= sh when you have several accounts using the same protocol (i.e. jabber). Anyway, I'd leave the UI fixing for later (and possibly to someone else, si= nce I really suck at that) and focus this review on the "core" of the displ= ay name stuff. - Daniele Elmo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103628/#review9534 ----------------------------------------------------------- On Jan. 4, 2012, 3:39 p.m., Daniele Elmo Domenichelli wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103628/ > ----------------------------------------------------------- > = > (Updated Jan. 4, 2012, 3:39 p.m.) > = > = > Review request for Telepathy. > = > = > Description > ------- > = > This patch makes the display name editable and let each plugin generate a= "default" display name, based on the parameters set. > - Default name is used if display name is empty or if it is the previous = default name. > - If the current display name _contains_ the old default display name, on= ly the default part is replaced. For example, if old default name is foo@ba= r.com and the display name is Foo when the user changes somet= hing and the new default display name is foo@buz.com, the new display name = becomes Foo (This happens realtime while the user is typing) > - Otherwise the user set display name is just left unchanged. > = > Also all the plugins were updated. For accounts without a specific plugin= , the display name is set automatically using the "account" parameter if it= exists. If it does not exist the user will have to add it manually, but th= is doesn't seem a big issue to me... > = > Branch here: http://quickgit.kde.org/?p=3Dclones%2Ftelepathy-accounts-kcm= %2Fddomenichelli%2Ftelepathy-accounts-kcm.git&a=3Dshortlog&h=3Drefs/heads/d= isplayname > = > (Note: Of course this patch won't be merged until master is re-opened) > = > = > This addresses bug 284930. > http://bugs.kde.org/show_bug.cgi?id=3D284930 > = > = > Diffs > ----- > = > plugins/butterfly/main-options-widget.h 3b1f264b043540303d8b1e7df846411= f78019de8 = > plugins/butterfly/main-options-widget.cpp 7e4800717673b6ee7d549bae7afbf= 3126e0a28d4 = > plugins/gabble/main-options-widget-facebook.h 4653b7304672716a41fa3f3af= c3f33fd39daf665 = > plugins/gabble/main-options-widget-facebook.cpp 7e800bef0c6a1bf99bbfea8= 4d3cb411c0ebac280 = > plugins/gabble/main-options-widget-googletalk.h b73df1129648dedb084af2d= 1f8ee056d06fa6bd4 = > plugins/gabble/main-options-widget-googletalk.cpp ed97bffb0dab256af5ab2= a17ad1279094e49092b = > plugins/gabble/main-options-widget-msn.h 75ccef3324aa07963f3816083133f7= cc38f79595 = > plugins/gabble/main-options-widget-msn.cpp 913a808810b2368850657bc4f76e= dfa4d7d1f681 = > plugins/gabble/main-options-widget.h 5443c28414c50945ea169fb585ba08b30d= 873169 = > plugins/gabble/main-options-widget.cpp 927bb32f1e29a7e4e7dbed64ed4eb7df= 56e96014 = > plugins/haze/aim-main-options-widget.h 2b98548eef266cb1b764a7b6f135d718= e8402c1a = > plugins/haze/aim-main-options-widget.cpp bc19d9ce83d8da6695551125ef83b9= 6aade3a9cf = > plugins/haze/icq-main-options-widget.h 89574e788c159f7a214d3360a6a6f28e= dd7684d9 = > plugins/haze/icq-main-options-widget.cpp b6dcbae0d419c732d9803b02410890= 774a683708 = > plugins/haze/msn-main-options-widget.h 56ffc3039f92e9d1912bc396ac85de33= 0b5ebf4f = > plugins/haze/msn-main-options-widget.cpp 3bab7912896db5fd7d08a5407f4ecf= 4a615a743a = > plugins/haze/myspaceim-main-options-widget.h 554fc5100d46b8730a89f15912= dd239a87938932 = > plugins/haze/myspaceim-main-options-widget.cpp 94e475de6f89d5095bbcc979= 8b144600042e87cb = > plugins/haze/skype-main-options-widget.h 5e7082ce186301762f9931568ed56c= 7109f828fa = > plugins/haze/skype-main-options-widget.cpp 87a3448601b2f338ca9d7361424b= 7a926c2b14ee = > plugins/haze/yahoo-main-options-widget.h 72a3b304152885641e91d7e148c1fc= 586076a8d2 = > plugins/haze/yahoo-main-options-widget.cpp a3e4f7cabfbd617f185d48a25ae0= 7aacd69e7a1d = > plugins/idle/main-options-widget.h 1f5120f0244fb3d811fd39bd33354d0b0499= 42d6 = > plugins/idle/main-options-widget.cpp f8aeb4b7bb777cc9b5973271344d1ebaac= 78aa52 = > plugins/rakia/rakia-main-options-widget.h bd1291b8318dbc468b005a54315a9= 365aaf35b0c = > plugins/rakia/rakia-main-options-widget.cpp ecb4ee4ef4b23869b3f73077e49= a5bd3ed7f035e = > plugins/salut/salut-main-options-widget.h ab51e1901340ab8630ba19e50b49c= 77eeb5ecd2d = > plugins/salut/salut-main-options-widget.cpp 64b41674482c25b77f0ae289213= db827dc820323 = > plugins/sunshine/sunshine-main-options-widget.h fbb6dca508d9bf7b313c4b1= 4dcd1c2097cc772d5 = > plugins/sunshine/sunshine-main-options-widget.cpp 21a74b76444416072291a= 929f5f3e10e63577793 = > src/KCMTelepathyAccounts/abstract-account-parameters-widget.h 0c6cd95db= 610b7cebebe3e06ca15b446fd7d024d = > src/KCMTelepathyAccounts/abstract-account-parameters-widget.cpp b61e836= b77809819a0f6f24f9b9b1bfe2a3af6a6 = > src/KCMTelepathyAccounts/account-edit-widget.h 5db8119dfae2a31e51fca60d= af40b6b083359f6f = > src/KCMTelepathyAccounts/account-edit-widget.cpp 312a4c913394c39e1b1319= 94fbc0a8d02b84d5df = > src/KCMTelepathyAccounts/account-edit-widget.ui 0ffd782fce025d430decd0b= 71dba36e5cf7422f0 = > src/KCMTelepathyAccounts/parameter-edit-widget.h 0e92bb7df35870778b339f= b83e2ddb232e9abb6f = > src/KCMTelepathyAccounts/parameter-edit-widget.cpp fc666c5c760ae31672fb= efc332217ff68a7cd633 = > src/add-account-assistant.cpp 3f189b440ddf26af4092af6ac11e247838087254 = > src/edit-account-dialog.cpp 37cb28faacbf63160eaa116924d015d5e59ed1ee = > src/salut-details-dialog.h 804d61b903bdf8c0946b1960aedac5b63b5b1b97 = > src/salut-details-dialog.cpp f95826dcaa7b1fda96a695bd5f23ba9a3d4ffa63 = > src/salut-enabler.h 7d4d640d19509024b9da269cb061ba43d2896dd9 = > src/salut-enabler.cpp 8cb1e03c9bb41df9f579ab15a319959b61a70e46 = > = > Diff: http://git.reviewboard.kde.org/r/103628/diff/diff > = > = > Testing > ------- > = > Created an account and modified the display name. > Edited an account and modified the display name. > Created salut account and edited using the dialog. > Edited salut account. > Tested most of the plugins. > More random tests. > = > = > Screenshots > ----------- > = > Editable display name > http://git.reviewboard.kde.org/r/103628/s/403/ > = > = > Thanks, > = > Daniele Elmo Domenichelli > = > --===============2111662900550832728== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/103628/

On January 4th, 2012, 3:52 p.m., Martin Kla= petek wrote:

By lookin=
g at the screenshot, it's not really clear what is that klineedit and w=
hat is it actually for. I quickly scrolled through the code and I saw no cl=
ick-message either, so I assume that the text will simply appear there (eve=
n letter by letter)? I'd like to see some label there. Also the icon lo=
oks wrong. I know you said it looks wrong on the right too, but currently i=
t looks just randomly placed, it doesn't align with the horizontal edge=
s/visual guidelines. How about moving the icon on the top right, making it =
bigger and putting big "Jabber" next to it (maybe bold? though th=
at's generaly bad) and another line below it saying something like &quo=
t;You're about to create a jabber account". Then make the display =
name part of the form below, but label it with something like "Account=
 name".

And don't forget that we're in freeze right now ;)

On January 4th, 2012, 3:53 p.m., Martin Klapetek wrote:

I meant t=
op left, not top right, sorry :)
The lineedi=
t has a "Placeholder Text" that at the moment shows "Display=
ed Name", so that's what the user will see when he adds a new acco=
unt, then yes, the text will appear letter by letter. The placeholder also =
appears if the string is empty. Moreover if the user leaves it empty he wil=
l receive an error. When the account is added, whatever the user chose is s=
hown in the main window.

You cannot set the text "You're about to create a jabber account&q=
uot;, because the same widget is used both for creating and editing the acc=
ount

In my opinion it must be clear to the user that the display name does _not_=
 belong to the form below, since is a personal preference and not a paramet=
er to connect to the account (that's why I used the frame). Moreover, t=
he org.freedesktop.Telepathy.Account interface supports setting the icon pr=
operty, so in the future we could let the user choose the icon or do someth=
ing like kopete that lets you choose the colour of the icon, useful to dist=
ingush when you have several accounts using the same protocol (i.e. jabber).

Anyway, I'd leave the UI fixing for later (and possibly to someone else=
, since I really suck at that) and focus this review on the "core"=
; of the display name stuff.

- Daniele Elmo


On January 4th, 2012, 3:39 p.m., Daniele Elmo Domenichelli wrote:

Review request for Telepathy.
By Daniele Elmo Domenichelli.

Updated Jan. 4, 2012, 3:39 p.m.

Descripti= on 

This patch makes the display name editable and let each plug=
in generate a "default" display name, based on the parameters set.
- Default name is used if display name is empty or if it is the previous de=
fault name.
- If the current display name _contains_ the old default display name, only=
 the default part is replaced. For example, if old default name is foo@bar.=
com and the display name is Foo <foo@bar.com> when the user changes s=
omething and the new default display name is foo@buz.com, the new display n=
ame becomes Foo <foo@buz.com> (This happens realtime while the user i=
s typing)
- Otherwise the user set display name is just left unchanged.

Also all the plugins were updated. For accounts without a specific plugin, =
the display name is set automatically using the "account" paramet=
er if it exists. If it does not exist the user will have to add it manually=
, but this doesn't seem a big issue to me...

Branch here: http://quickgit.kde.org/?p=3Dclones%2Ftelepathy-accounts-kcm%2=
Fddomenichelli%2Ftelepathy-accounts-kcm.git&a=3Dshortlog&h=3Drefs/h=
eads/displayname

(Note: Of course this patch won't be merged until master is re-opened)

Testing <= /h1> 
Created an account and modified the display name.
Edited an account and modified the display name.
Created salut account and edited using the dialog.
Edited salut account.
Tested most of the plugins.
More random tests.
Bugs: 284930

Diffs=

  • plugins/butterfly/main-options-widget.h (3= b1f264b043540303d8b1e7df846411f78019de8)
  • plugins/butterfly/main-options-widget.cpp = (7e4800717673b6ee7d549bae7afbf3126e0a28d4)
  • plugins/gabble/main-options-widget-facebook.h (4653b7304672716a41fa3f3afc3f33fd39daf665)
  • plugins/gabble/main-options-widget-facebook.cpp (7e800bef0c6a1bf99bbfea84d3cb411c0ebac280)
  • plugins/gabble/main-options-widget-googletalk.h (b73df1129648dedb084af2d1f8ee056d06fa6bd4)
  • plugins/gabble/main-options-widget-googletalk.cpp (ed97bffb0dab256af5ab2a17ad1279094e49092b)
  • plugins/gabble/main-options-widget-msn.h (= 75ccef3324aa07963f3816083133f7cc38f79595)
  • plugins/gabble/main-options-widget-msn.cpp (913a808810b2368850657bc4f76edfa4d7d1f681)
  • plugins/gabble/main-options-widget.h (5443= c28414c50945ea169fb585ba08b30d873169)
  • plugins/gabble/main-options-widget.cpp (92= 7bb32f1e29a7e4e7dbed64ed4eb7df56e96014)
  • plugins/haze/aim-main-options-widget.h (2b= 98548eef266cb1b764a7b6f135d718e8402c1a)
  • plugins/haze/aim-main-options-widget.cpp (= bc19d9ce83d8da6695551125ef83b96aade3a9cf)
  • plugins/haze/icq-main-options-widget.h (89= 574e788c159f7a214d3360a6a6f28edd7684d9)
  • plugins/haze/icq-main-options-widget.cpp (= b6dcbae0d419c732d9803b02410890774a683708)
  • plugins/haze/msn-main-options-widget.h (56= ffc3039f92e9d1912bc396ac85de330b5ebf4f)
  • plugins/haze/msn-main-options-widget.cpp (= 3bab7912896db5fd7d08a5407f4ecf4a615a743a)
  • plugins/haze/myspaceim-main-options-widget.h (554fc5100d46b8730a89f15912dd239a87938932)
  • plugins/haze/myspaceim-main-options-widget.cpp (94e475de6f89d5095bbcc9798b144600042e87cb)
  • plugins/haze/skype-main-options-widget.h (= 5e7082ce186301762f9931568ed56c7109f828fa)
  • plugins/haze/skype-main-options-widget.cpp (87a3448601b2f338ca9d7361424b7a926c2b14ee)
  • plugins/haze/yahoo-main-options-widget.h (= 72a3b304152885641e91d7e148c1fc586076a8d2)
  • plugins/haze/yahoo-main-options-widget.cpp (a3e4f7cabfbd617f185d48a25ae07aacd69e7a1d)
  • plugins/idle/main-options-widget.h (1f5120= f0244fb3d811fd39bd33354d0b049942d6)
  • plugins/idle/main-options-widget.cpp (f8ae= b4b7bb777cc9b5973271344d1ebaac78aa52)
  • plugins/rakia/rakia-main-options-widget.h = (bd1291b8318dbc468b005a54315a9365aaf35b0c)
  • plugins/rakia/rakia-main-options-widget.cpp = (ab51e1901340ab8630ba19e50b49c77eeb5ecd2d)
  • plugins/salut/salut-main-options-widget.cpp (fbb6dca508d9bf7b313c4b14dcd1c2097cc772d5)
  • plugins/sunshine/sunshine-main-options-widget.cpp (21a74b76444416072291a929f5f3e10e63577793)
  • src/KCMTelepathyAccounts/abstract-account-parameters-widget.h (0c6cd95db610b7cebebe3e06ca15b446fd7d024d)
  • src/KCMTelepathyAccounts/abstract-account-parameters-widget.cpp (b61e836b77809819a0f6f24f9b9b1bfe2a3af6a6)
  • src/KCMTelepathyAccounts/account-edit-widget.h (5db8119dfae2a31e51fca60daf40b6b083359f6f)
  • src/KCMTelepathyAccounts/account-edit-widget.cpp (312a4c913394c39e1b131994fbc0a8d02b84d5df)
  • src/KCMTelepathyAccounts/account-edit-widget.ui (0ffd782fce025d430decd0b71dba36e5cf7422f0)
  • src/KCMTelepathyAccounts/parameter-edit-widget.h (0e92bb7df35870778b339fb83e2ddb232e9abb6f)
  • src/KCMTelepathyAccounts/parameter-edit-widget.cpp (fc666c5c760ae31672fbefc332217ff68a7cd633)
  • src/add-account-assistant.cpp (3f189b440dd= f26af4092af6ac11e247838087254)
  • src/edit-account-dialog.cpp (37cb28faacbf6= 3160eaa116924d015d5e59ed1ee)
  • src/salut-details-dialog.h (804d61b903bdf8= c0946b1960aedac5b63b5b1b97)
  • src/salut-details-dialog.cpp (f95826dcaa7b= 1fda96a695bd5f23ba9a3d4ffa63)
  • src/salut-enabler.h (7d4d640d19509024b9da2= 69cb061ba43d2896dd9)
  • src/salut-enabler.cpp (8cb1e03c9bb41df9f57= 9ab15a319959b61a70e46)

View Diff

Screensho= ts

3D"Editable
--===============2111662900550832728==-- --===============1706867191248347346== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy --===============1706867191248347346==--