----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/4119/#review5858 -----------------------------------------------------------
what is this patch fixing in the current pager? the checkbox is precisely how we do them in plasma, and the reasons for this is a combination of visual scanability and aesthetics. also, since we're in string freeze, we can't change strings and "Display icons:" with a ':' looks (and is) incorrect when placed to the right of the checkbox. why is the configure desktops button so large? honestly, if i were to suggest an improvement it would be this: get rid of the Configure Desktops button and simply include the desktops kcm in the dialog directly as a new page, just as we do elsewhere for, e.g., the automount and device action kcms in the device notifier plasmoid. - Aaron On 2010-05-23 22:52:04, Ignat Semenov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/4119/ > ----------------------------------------------------------- > > (Updated 2010-05-23 22:52:04) > > > Review request for Plasma. > > > Summary > ------- > > This is the final version of the Pager configuration Dialog patch. Changes: > > - Brought back radiobuttons > - Changed the "Display icons" checkbox to respect KDE style > - Introduced a ton of layouts and set a top-level layout so that the dialog > scales properly now > - Centered the "Configure Desktops" button and made it huge > > I plan to fix the layout issues for all plasmoid configuration dialogs, as > time allows. > Now I'd like to know what's the rationale behind headers in configuration > dialogs. They're redundant from my point of view as they duplicate the > information which is on the left in the list of configuration pages. Maybe > they can be removed? Not it Plasma, of course, but in KConfigurationDialog > source. > > > Diffs > ----- > > /trunk/KDE/kdebase/workspace/plasma/desktop/applets/pager/pager.cpp 1129710 > /trunk/KDE/kdebase/workspace/plasma/desktop/applets/pager/pagerConfig.ui > 1129710 > > Diff: http://reviewboard.kde.org/r/4119/diff > > > Testing > ------- > > Built it and it really scales properly now. There is only a minor problem > with Oxygen style and I've already filed a bug against Oxygen. > > > Screenshots > ----------- > > New configuration dialog > http://reviewboard.kde.org/r/4119/s/410/ > > > Thanks, > > Ignat > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel