> On Jan. 4, 2012, 3:52 p.m., Martin Klapetek wrote: > > By looking at the screenshot, it's not really clear what is that klineedit > > 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 there > > (even letter by letter)? I'd like to see some label there. Also the icon > > looks wrong. I know you said it looks wrong on the right too, but currently > > it looks just randomly placed, it doesn't align with the horizontal > > edges/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 > > generaly 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 :) > > Daniele Elmo Domenichelli wrote: > The lineedit has a "Placeholder Text" that at the moment shows "Displayed > Name", 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 the > 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 > main 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 > parameter to connect to the account (that's why I used the frame). Moreover, > the org.freedesktop.Telepathy.Account interface supports setting the icon > property, 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 > distingush 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. >
Totally love the plugin's setting the default name. +10 from me on that front. Though I'm also unsure on the UI, and think the code is a bit over-clever with the dynamic updating. (Personally I don't think we really need it editable once we have the plugins are setting sensible names. We rarely actually show the account name. However I'm not against it being in if it's not confusing.) Trying to be impartial I did a pop UI quiz (albeit on the screenshot not working code) with a Gnome Pidgin user: < What do you think that top line edit is for. < (the one just underneath the word "Jabber") > At first I had no idea (you can tell it's 5 am), but I would guess nickname, > since there seems to be a picture placeholder next to it. (Oh, and I'm not a > KDE user, so I might miss any conventions) < so your name as you appear to other users? > Yeah Will try again with my housemate's with a working version where they have the placeholder text. If they cope fine, then +1 from me. If they still get confused too, we need to change something. I'm not sure we can rely on placeholder text, it's not there when you edit, and you can't expect someone to remember the helptext they saw when they created the account 6 months ago. Even if it's a label "Local Alias" (not a fan, it's a bit technical). Or we don't have the edit field here, and/or put a rename in the context menu in the main view. Or it's a label which you can click to edit Or a button to change it Or something I've not thought of. - David ----------------------------------------------------------- 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, only > the default part is replaced. For example, if old default name is > [email protected] and the display name is Foo <[email protected]> when the user changes > something and the new default display name is [email protected], the new display > name becomes Foo <[email protected]> (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 this > doesn't seem a big issue to me... > > Branch here: > http://quickgit.kde.org/?p=clones%2Ftelepathy-accounts-kcm%2Fddomenichelli%2Ftelepathy-accounts-kcm.git&a=shortlog&h=refs/heads/displayname > > (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=284930 > > > Diffs > ----- > > plugins/butterfly/main-options-widget.h > 3b1f264b043540303d8b1e7df846411f78019de8 > 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 > 5443c28414c50945ea169fb585ba08b30d873169 > plugins/gabble/main-options-widget.cpp > 927bb32f1e29a7e4e7dbed64ed4eb7df56e96014 > plugins/haze/aim-main-options-widget.h > 2b98548eef266cb1b764a7b6f135d718e8402c1a > plugins/haze/aim-main-options-widget.cpp > bc19d9ce83d8da6695551125ef83b96aade3a9cf > plugins/haze/icq-main-options-widget.h > 89574e788c159f7a214d3360a6a6f28edd7684d9 > plugins/haze/icq-main-options-widget.cpp > b6dcbae0d419c732d9803b02410890774a683708 > plugins/haze/msn-main-options-widget.h > 56ffc3039f92e9d1912bc396ac85de330b5ebf4f > 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 1f5120f0244fb3d811fd39bd33354d0b049942d6 > plugins/idle/main-options-widget.cpp > f8aeb4b7bb777cc9b5973271344d1ebaac78aa52 > plugins/rakia/rakia-main-options-widget.h > bd1291b8318dbc468b005a54315a9365aaf35b0c > plugins/rakia/rakia-main-options-widget.cpp > ecb4ee4ef4b23869b3f73077e49a5bd3ed7f035e > plugins/salut/salut-main-options-widget.h > ab51e1901340ab8630ba19e50b49c77eeb5ecd2d > plugins/salut/salut-main-options-widget.cpp > 64b41674482c25b77f0ae289213db827dc820323 > plugins/sunshine/sunshine-main-options-widget.h > 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 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 > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
