> On July 26, 2011, 10:46 p.m., Aleix Pol Gonzalez wrote: > > kdeui/widgets/klineedit.cpp, line 358 > > <http://git.reviewboard.kde.org/r/102095/diff/1/?file=30032#file30032line358> > > > > Wouldn't it be better to put it this way? Just saying... > > > > d->clearButton->animateVisible(d->wideEnoughForClear && text.length() > > > 0);
I think the original is clearer, to be honest. - Nicolas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102095/#review5129 ----------------------------------------------------------- On July 26, 2011, 9:54 p.m., Hugo Pereira Da Costa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102095/ > ----------------------------------------------------------- > > (Updated July 26, 2011, 9:54 p.m.) > > > Review request for kdelibs. > > > Summary > ------- > > Details: > - fixes the somewhat incorrect logic in KLineEditButton::animateVisible > - simplifies KLineEdit::updateClearButtonIcon consequently. > > > This addresses bug 268898. > http://bugs.kde.org/show_bug.cgi?id=268898 > > > Diffs > ----- > > kdeui/widgets/klineedit.cpp 8f1c8a4 > kdeui/widgets/klineedit_p.h 95016bd > > Diff: http://git.reviewboard.kde.org/r/102095/diff > > > Testing > ------- > > tested with klineedittest found in kdelibs/kdeui/tests, this with and without > the patch attached to comment #1 of bug 268898, used to actually trigger the > mentionned bug. Also tested with other klineEdit implementation such as > Dolphin's location bar. > > > Thanks, > > Hugo > >
