> On April 26, 2012, 12:15 p.m., Marco Martin wrote: > > i agree that looks better here, but i would use thins only for the > > toolbutton (and not doing a similar thing for pushbuttons) > > > > the reason is in part aestetic (if they are stacked vertically they look > > better if they have the same width) and in part technical: plasma will try > > to share the rendered svgs both in memory and in disk cache, so the more > > elements with exactly the same size there are, the more memory is saved > > Aurélien Gâteau wrote: > I am not sure what you mean with "only for the toolbutton": this patch > only touches ToolButton. > > I agree a column of buttons look nicer if they have the same width, but > defining a minimum width in ToolButton is not the correct way to go: if the > text of some buttons is longer than the minimum width, those buttons won't be > correctly aligned with the others (and you can't rely on the text staying > short enough once translated). To ensure all widgets in a column to have the > same width, you have to enforce it at the column level. > > Regarding the memory reason: I believe internal optimizations should not > dictate the appearance of the UI, so I don't think this is a valid reason.
what i said is that this patch is ok as is touches only toolbutton, i just wouldn't want a similar one for pushbutton. as for the memory, right now decreasing memory usage is my #1 priority, we simply can't forget that constraints do exist, sorry. - Marco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104735/#review12925 ----------------------------------------------------------- On April 26, 2012, 8:27 a.m., Aurélien Gâteau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104735/ > ----------------------------------------------------------- > > (Updated April 26, 2012, 8:27 a.m.) > > > Review request for Plasma and David Edmundson. > > > Description > ------- > > This patch fixes two issues in ToolButton layout: > > 1. Make sure there is some space between the button icon and its text > > 2. Do not enforce a minimum width > > The reason for #2 is that having a minimum width does not make much sense for > a ToolButton: > - It should aim at using the minimum amount of horizontal space when used in > a ToolBar. > - It looks unbalanced when used with an icon because the content is flushed > to the left, leaving a large amount of white-space on the right. (See > attached screenshots) > > > Diffs > ----- > > plasma/declarativeimports/plasmacomponents/qml/ToolButton.qml 0447a69 > > Diff: http://git.reviewboard.kde.org/r/104735/diff/ > > > Testing > ------- > > Run attached test script, you should notice the differences in spacing > between button icon and text, as well as in the white-space on the right of > the button. > > > Screenshots > ----------- > > Before > http://git.reviewboard.kde.org/r/104735/s/545/ > After > http://git.reviewboard.kde.org/r/104735/s/546/ > > > Thanks, > > Aurélien Gâteau > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
