Opened 13 years ago

Closed 11 years ago

Last modified 11 years ago

#619 closed bug (fixed)

[BListView] selection should be centered

Reported by: diver Owned by: axeld
Priority: normal Milestone: R1
Component: Kits/Interface Kit Version:
Keywords: Cc: cl21
Blocked By: Blocking:
Has a Patch: no Platform: All


Screenshot of what i mean will follow :-)

Attachments (2)

BListView.PNG (25.0 KB) - added by diver 13 years ago.
patch-bug#619.diff (689 bytes) - added by cl21 11 years ago.
New patch

Download all attachments as: .zip

Change History (9)

comment:1 Changed 13 years ago by axeld

You mean the text should be centered vertically in the selection box, right? (just to be sure :-))

comment:2 Changed 13 years ago by diver

Yep :-)

Changed 13 years ago by diver

Attachment: BListView.PNG added


comment:3 Changed 12 years ago by axeld

Platform: All

The patch definitely improves the situation, however it is not entirely correct, either. Since the ascent and descent are the maximal height of the font, you would need to ceilf() either of them individually. As you can see, the SetHeight() call after your changes does it incorrectly as well. You should also ceil() or floor() the leading/2 and double that exact value for SetHeight(). Do you want to do these changes and apply another patch, or should I rework your patch and commit it?

comment:4 Changed 11 years ago by diver

Cc: cl21 added
Component: - GeneralKits/Kernel Kit

cl21, could you take a look at this bug again?

Changed 11 years ago by cl21

Attachment: patch-bug#619.diff added

New patch

comment:5 Changed 11 years ago by cl21

Axel, do you mean like this (see attached patch)? When testing this it does not look right to me for characters with a lot of descent such as "q". They do not seem to have enough space underneath.

comment:6 Changed 11 years ago by axeld

Component: Kits/Kernel KitKits/Interface Kit
Resolution: fixed
Status: newclosed

Thanks for the patch! Fixed in hrev23046. I've changed the patch a bit as I shared your experience - even though it may not be as mathematically accurate than your version, at least it now looks best.

comment:7 Changed 11 years ago by diver

Hey cl21, could you please take a look at a similar bug #1319

Note: See TracTickets for help on using tickets.