Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#10286 closed defect (fixed)

improve wxCheckListBox appearance on MSW

Reported by: malcompl Owned by:
Priority: normal Milestone:
Component: wxMSW Version:
Keywords: wxCheckListBox Cc:
Blocked By: Blocking:
Patch: yes

Description

Checkbox positions are mismatched and wxCheckListBox looks very bad on Vista, so I've fixed it (the patch looks like continuation of #8488).
Cleand-up code, deleted magic number, fixed calculation checkmarks position, fixed small bug on check last item if it isn't visible at all, and some small corrections.

Now, it's working well and does have nice apperance on Vista, XP, and other windows ;)

Attachments (1)

checklst.patch download (24.8 KB) - added by malcompl 5 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by vadz

  • Status changed from new to confirmed
  • Summary changed from wxCheckListBox draw correction on MSW to improve wxCheckListBox appearance on MSW

Thanks for the patch! It does look good but I wonder if we can do something better/more maintainable about the corrections you apply to compensate for wxOwnerDrawn::OnDrawItem() logic, e.g. by adding some virtual function returning the margin to use which would allow us to have different behaviours for wxMenuItem and wxCheckListBox?

Otherwise the only comment I have is that we already have wxRectFromRECT() in wx/msw/private.h which could be used in wxListBox::GetItemRect().

Thanks!

comment:2 follow-up: Changed 6 years ago by malcompl

I understand it, but some msw part still using other method, not wxRectFromRECT() for convert RECT to wxRect.

IMHO would be better if wxOwnerDrawn will be a base class for specified ownerdrawn item's class. That it's curently, but the class should provide only drawing text label (with margin, color, font), without drawing bitmaps (it's should be moved to derived classes). Then use wxOwnerDrawn will be more flexible.
Vadim, what you think about it?

comment:3 in reply to: ↑ 2 Changed 6 years ago by vadz

Replying to malcompl:

I understand it, but some msw part still using other method, not wxRectFromRECT() for convert RECT to wxRect.

Yes, wxRectFromRECT() is a relatively "recent" addition (a few years old) and all code written before then it doesn't use it. But I think it's nice to use it in the new code at least.

IMHO would be better if wxOwnerDrawn will be a base class for specified ownerdrawn item's class. That it's curently, but the class should provide only drawing text label (with margin, color, font), without drawing bitmaps (it's should be moved to derived classes). Then use wxOwnerDrawn will be more flexible.
Vadim, what you think about it?

This is possible and is better than what I proposed if there is really nothing in common between drawing bitmaps for the different owner drawn items. I don't think this is true though, I'd expect most owner drawn controls to look the same or very similar, it's just the menus which are special. So IMHO just moving some of the code currently in wxOwnerDrawn itself into virtual functions in it would be a better solution (and also a simpler one, i.e. requiring less changes) but I'm leaving the decision to you as you're doing the real work here. I just would like to avoid making the assumptions like the current patch does about wxOwnerDrawn always adding 4 pixels margin and stuff like this because it will inevitably break in the future.

Thanks!

comment:4 Changed 6 years ago by vadz

  • Status changed from confirmed to infoneeded_new

Changing status as I think that the patch does need to be modified in either one or the other way.

TIA!

comment:5 Changed 5 years ago by malcompl

  • Status changed from infoneeded_new to new

Patch modified to change from #10635
I updated apperance, drawing check item, recalculate some sizes and coordinates, moved drawing focus rect to base class - wxOwnerDrawn::OnDrawItem(), improvement other case in wxCheckListBox and base class - wxListBox.

Next patch will be fix margin alignment on menu item.

Changed 5 years ago by malcompl

comment:6 Changed 5 years ago by vadz

  • Status changed from new to confirmed

Thanks, I'll apply it soon but I'd be really grateful if you could make the patches smaller as this one was very difficult to review. In particular, please don't make whitespace changes, renaming (stuff like m_bChecked to m_check, Set to SET and so on) in the main patch. If you really want to do it, please submit a separate patch which clearly does nothing else but changes indentation or renames something.

TIA!

comment:7 Changed 5 years ago by malcompl

Ok, I understand, but in this patch I must rename enum values to avoid name conflict with some new method added by patch.

comment:8 Changed 5 years ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [62715]) Improve wxCheckListBox appearance under Vista/Win7.

Fix the items alignment and also code cleanup: fix indentation, remove magic
numbers &c.

Closes #10286.

comment:9 Changed 5 years ago by VZ

(In [63226]) Improve wxCheckListBox appearance under Vista/Win7.

Fix the items alignment and also code cleanup: fix indentation, remove magic
numbers &c.

Closes #10286.

Note: See TracTickets for help on using tickets.