----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107813/#review23723 -----------------------------------------------------------
Ship it! I like this, it cleans up some "why is this that way anyway" stuff, makes the code much cleaner. I haven't tested it, but it *looks* good to me. 4.9, 4.10 and master branches, please. - Sebastian Kügler On Dec. 19, 2012, 1:46 p.m., Sebastian Gottfried wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107813/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2012, 1:46 p.m.) > > > Review request for KDE Runtime and Marco Martin. > > > Description > ------- > > There is a bug in the tool button component resulting in layout breakage if > one clears and sets the text property of the component when not visible, see > the attached screenshot. > > I have tried to solve the issue without changing the existing anchoring > system, but without success. The only working solution was to put the icon > and the label item into Column item. That way I was able to fix the bug and > even get rid of the ugly explicit non-declarative anchor assignments. > > I have also removed the preferredWidth property of the label item, that one > always evaluated to paintedWidth, anyway. > > For consistency the button component should get the same treatment, I will do > so once I get positive feedback here. > > > Diffs > ----- > > plasma/declarativeimports/plasmacomponents/qml/ToolButton.qml 594067d > > Diff: http://git.reviewboard.kde.org/r/107813/diff/ > > > Testing > ------- > > Work as expected. No behavioral changes apart from the bugfix. > > > Screenshots > ----------- > > Broken tool button layout > http://git.reviewboard.kde.org/r/107813/s/921/ > > > Thanks, > > Sebastian Gottfried > >
