[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request: [Quicklaunch] Refactoring of the Quicklaunch
From:       "Ingomar Wesp" <ingomar () wesp ! name>
Date:       2010-04-26 11:11:43
Message-ID: 20100426111143.17793.18896 () localhost
[Download RAW message or body]



> On None, Lukas Appelhans wrote:
> > Hey!
> > 
> > Great work! I had a look over the code and tested it and it works greatly!
> > 
> > 2 things:
> > Why do we use a custom layout instead of a QGridLayout?
> > And I think the column setting is unnecessary... no? :)
> > 
> > Anyway, as the freeze is near, I added an entry to the feature plan about this...
> > 
> > I would say +1 for committing after the 2 things have been discussed...
> > 
> > Lukas

Hi!

> Great work! I had a look over the code and tested it and it works greatly!
Thanks, I'm happy to hear that.

> Why do we use a custom layout instead of a QGridLayout?

2 reasons, primarily.
For one, I didn't want to have to completely repopulate the underlying \
QGraphicsGridLayout every time the icon wrapping changes (caused by resizes or \
changes to the applet's configuration). Secondly, I thought that the code is simpler \
to understand if one doesn't need to know the exact behavior of QGraphicsGridLayout \
(with which I also had a few problems with regards to row/column spacing IIRC).

> And I think the column setting is unnecessary... no? :)

I'm afraid I'm not sure what you are referring to.
Do you mean the ability to set the maximum number of columns in vertical formfactors?

> Anyway, as the freeze is near, I added an entry to the feature plan about this...

Ah, great. Thanks a lot!

Best regards,
Ingo


- Ingomar


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


On 2010-04-23 19:08:34, Ingomar Wesp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3786/
> -----------------------------------------------------------
> 
> (Updated 2010-04-23 19:08:34)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This is my proposed patch for the refactored quicklaunch applet as discussed in \
> review request http://reviewboard.kde.org/r/3358/ and on the ML a while ago. Sorry \
> that it took so long... 
> Summary of changes:
> - Refactored the quicklaunch applet, so that the applet,
> icon grid widget and icon grid layout are split into
> separate classes all living in a newly created namespace.
> 
> - Improved drag & drop behaviour (it is not possible to drop items
> in the popup dialog) and drag & drop markers.
> 
> - Icons are now moved to/from the dialog explicitly instead of
> asking the user to specify the number of the icons that are
> shown in the primary area.
> 
> - Icon size is now determined automatically based on the
> available space, hard-coded minimum and maximum bounds and
> the number of rows (or columns) set by the user. This is done
> in a custom layout that is no longer based on
> QGraphicsGridLayout.
> 
> - When all icons are removed from an icon area, a placeholder
> icon is displayed.
> 
> As this patch changes the configuration keys used, it also incorporates code for \
> migrating older config keys. 
> Unfortunately, using svn diff with files that have been "svn move"d appears to \
> yield broken diffs, so the patch here does not reflect the history for files that \
> are based on renamed files (quicklaunch.*, quicklaunchicon.*), but I'll make sure \
> that the history is preserved when committing (if this gets a "ship it", that is). 
> Please give it a spin and tell me what you think.
> 
> 
> This addresses bugs 206382, 206912, 214463, 225011, and 233914.
> https://bugs.kde.org/show_bug.cgi?id=206382
> https://bugs.kde.org/show_bug.cgi?id=206912
> https://bugs.kde.org/show_bug.cgi?id=214463
> https://bugs.kde.org/show_bug.cgi?id=225011
> https://bugs.kde.org/show_bug.cgi?id=233914
> 
> 
> Diffs
> -----
> 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/CMakeLists.txt \
>                 1117710 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/QuicklaunchLayout.h \
>                 1117710 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/QuicklaunchLayout.cpp \
>                 1117710 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/icongrid.h \
>                 PRE-CREATION 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/icongrid.cpp \
>                 PRE-CREATION 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/icongridlayout.h \
>                 PRE-CREATION 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/icongridlayout.cpp \
>                 PRE-CREATION 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/quicklaunch.h \
>                 PRE-CREATION 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/quicklaunch.cpp \
>                 PRE-CREATION 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/quicklaunchApplet.h \
>                 1117710 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/quicklaunchApplet.cpp \
>                 1117710 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/quicklaunchConfig.ui \
>                 1117710 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/quicklaunchIcon.h \
>                 1117710 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/quicklaunchIcon.cpp \
>                 1117710 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/quicklaunchicon.h \
>                 PRE-CREATION 
> /trunk/KDE/kdebase/workspace/plasma/generic/applets/quicklaunch/quicklaunchicon.cpp \
> PRE-CREATION  
> Diff: http://reviewboard.kde.org/r/3786/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ingomar
> 
> 

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

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