--===============3911823482032299232== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Oct. 5, 2011, 11:30 a.m., David Faure wrote: > > Why the setMaxLength?? What if one wants to type in a long URL? > > = > > Also, I can't reproduce the bug here (kde-4.7), but maybe only because = the big icon button makes the dialog quite large? > = > Greg T wrote: > indeed, the setmaxLength was stupid > = > it's quite small on 4.6.5 http://wstaw.org/m/2011/10/05/plasma-deskto= pF27745.jpg > = > > = > David Faure wrote: > Ah I see, it's the german translation which makes the label longer ("= Choose an icon" -> "W=C3=A4hlen Sie ein Symbol aus"), leaving less room for= the lineedits. > = > Patch 3 looks good, although I can't help but think there's a bug if = a set*Width() call takes a height() as value :-) But I'll trust Christoph o= n this one. > = > Peter Penz wrote: > > Patch 3 looks good, although I can't help but think there's a bug i= f a > > set*Width() call takes a height() as value :-) But I'll trust Chris= toph on this one. > = > Using QFontMetrics::averageCharWidth() might be an alternative in com= parison to use the height as reference for calculating the width (but proba= bly this has some drawbacks I'm not aware of). First, averageCharWidth() is slow. It goes through every character to compu= te the average. Second, that average might be skewed when there are broken = characters in the font. I'll add a comment why the lineedits width depends on the fonts height. - Christoph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102751/#review7117 ----------------------------------------------------------- On Oct. 5, 2011, 6:22 p.m., Greg T wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102751/ > ----------------------------------------------------------- > = > (Updated Oct. 5, 2011, 6:22 p.m.) > = > = > Review request for Dolphin and kdelibs. > = > = > Description > ------- > = > Hi, this patch sets a minimum size for the widget. = > = > = > This addresses bug 266435. > http://bugs.kde.org/show_bug.cgi?id=3D266435 > = > = > Diffs > ----- > = > kfile/kfileplaceeditdialog.cpp d798b4d = > = > Diff: http://git.reviewboard.kde.org/r/102751/diff/diff > = > = > Testing > ------- > = > it's working > = > = > Thanks, > = > Greg T > = > --===============3911823482032299232== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable

This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/102751/

On October 5th, 2011, 11:30 a.m., David Fau= re wrote:

Why the s=
etMaxLength?? What if one wants to type in a long URL?

Also, I can't reproduce the bug here (kde-4.7), but maybe only because =
the big icon button makes the dialog quite large?

On October 5th, 2011, 6:23 p.m., Greg T wrote:

indeed, t=
he setmaxLength was stupid

it's quite small on 4.6.5 http://wstaw.org/m/2011/10/05/plasma-desktopF=
27745.jpg

On October 6th, 2011, 7:52 a.m., David Faure wrote:

Ah I see,=
 it's the german translation which makes the label longer ("Choose=
 an icon" -> "W=C3=A4hlen Sie ein Symbol aus"), leaving l=
ess room for the lineedits.

Patch 3 looks good, although I can't help but think there's a bug i=
f a set*Width() call takes a height() as value :-) But I'll trust Chris=
toph on this one.

On October 6th, 2011, 8:15 a.m., Peter Penz wrote:

> Patc=
h 3 looks good, although I can't help but think there's a bug if a
> set*Width() call takes a height() as value :-) But I'll trust Chri=
stoph on this one.

Using QFontMetrics::averageCharWidth() might be an alternative in compariso=
n to use the height as reference for calculating the width (but probably th=
is has some drawbacks I'm not aware of).

First, aver=
ageCharWidth() is slow. It goes through every character to compute the aver=
age. Second, that average might be skewed when there are broken characters =
in the font.

I'll add a comment why the lineedits width depends on the fonts height.=

- Christoph

On October 5th, 2011, 6:22 p.m., Greg T wrote:

Review request for Dolphin and kdelibs.

By Greg T.

Updated Oct. 5, 2011, 6:22 p.m.

Descripti= on

Hi, this patch sets a minimum size for the widget.

Testing <= /h1>

it's working

Bugs: 266435

Diffs=

kfile/kfileplaceeditdialog.cpp (d798b4d)

--===============3911823482032299232==--