-----------------------------------------------------------
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
>
>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
