Review Request: Place the user avatar next to the user/computer info in Kickoff

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/870/#review1364
-----------------------------------------------------------



at one point i spent a lot of time lining up _all_ the items in kickoff so that there were as few vertical alignment points as possible and so that things always aligned properly.

apparently in current svn the search icon is now bigger again and the "Search:" label doesn't line up. *sigh*

in any case, your patch makes it even a bit worse, and the top header area is now a complete jumble.

here's my suggestions:

* put some spacing below the user icon so it's not so close to the content area

* align the left of the user icon with left of the content area (it seems a bit shifted to the right right now?)

* get rid of the icon next to search, it's not needed

* put the Search: label inside the lineedit: setClickMessage(i18n("Search")); this will align the search with the user name (with the text below, too). this introduces a small complexity, however: for the click message to show, we can't give it focus. but we want the user to be able to just type away and search. there's an Launcher::eventFilter method in kickoff/ui/launcher.cpp that would need som tricks added to it to make this work properly (essentially, when the user starts typing and it isn't a navigation key like an arrow and a modifier key isn't pressed, set the text in the search box and give it focus)

other comments are in-line to the patch.


/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp
<http://reviewboard.kde.org/r/870/#comment867>

    this should be using KUser's faceIconPath() method.



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp
<http://reviewboard.kde.org/r/870/#comment868>

    the size really should be based on the width of the column; see the static ints in ui/itemdelegate.h



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp
<http://reviewboard.kde.org/r/870/#comment869>

    this breaks the vertical layout. see above comments



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp
<http://reviewboard.kde.org/r/870/#comment870>

    and again, breaking the visual layout.


- Aaron


On 2009-06-22 09:24:13, Jonathan Thomas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/870/
> -----------------------------------------------------------
> 
> (Updated 2009-06-22 09:24:13)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> In the KDE kickoff menu, the avatar currently appears next to the menu search bar. It should be next to the username since the avatar and username are related information. See screenshot.
> (This is a seele-recommended fix, see https://launchpad.net/bugs/389744 )
> Here's the before picture: http://launchpadlibrarian.net/28128348/avatar.png
> 
> The patch basically moves the search for the user icon to launcher.cpp, and places either the found user icon-- or the generic user-identity icon if none is found-- next to the user/computer info. The search icon has been made smaller so that the new layout uses just as much space as the old layout.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/searchbar.cpp 978774 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 978774 
> 
> Diff: http://reviewboard.kde.org/r/870/diff
> 
> 
> Testing
> -------
> 
> I have tested it. It's a fairly straightforward patch.
> 
> 
> Screenshots
> -----------
> 
> After
>   http://reviewboard.kde.org/r/870/s/134/
> 
> 
> Thanks,
> 
> Jonathan
> 
>