> 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
