Opened 6 years ago

Closed 6 years ago

Last modified 18 months ago

#9591 closed enhancement (fixed)

Item state (icons) for wxTreeCtrl on any platform

Reported by: malcompl Owned by:
Priority: normal Milestone:
Component: GUI-all Version:
Keywords: wxTreeCtrl Cc:
Blocked By: Blocking:
Patch: yes

Description

The item state icons are supported by any platform now ;)

Attachments (8)

treestate.patch download (21.4 KB) - added by malcompl 6 years ago.
treedocs.patch download (1.2 KB) - added by malcompl 6 years ago.
treesample.patch download (27.1 KB) - added by malcompl 6 years ago.
treecorrects.patch download (5.1 KB) - added by malcompl 6 years ago.
treesampletransp.patch download (3.0 KB) - added by malcompl 6 years ago.
treerect.patch download (4.2 KB) - added by malcompl 6 years ago.
treeedit.patch download (1.9 KB) - added by malcompl 6 years ago.
resetbroken.patch download (2.4 KB) - added by malcompl 6 years ago.

Download all attachments as: .zip

Change History (32)

Changed 6 years ago by malcompl

comment:1 Changed 6 years ago by roebling

  • Status changed from new to infoneeded_new

A very interesting patch. I wonder what item_state_next and item_state_prev means? The natural thing would be to have a number for the state, next and previous require explanation (to me).

comment:2 Changed 6 years ago by malcompl

  • Status changed from infoneeded_new to new

Normally we use a number for the state, but in some situations, if we change the state to current state +/- 1, we don't have to get the current state, in/decrement and set it again, we only need to use SetItemState and item_state_next or item_state_prev.

Another example:
We have 2 states (0 and 1), every click on the state icon should change the current state into its reverse (checkbox), then in the event handling we only need to use SetItemState with a value item_state_next, instead of get the current state, and set it reverse using 'if'.

The inspiration for these two 'special' values was -1 (cycle to the next one) in wxTreectrl::SetState in the previous wxMSW implementation.

comment:3 Changed 6 years ago by roebling

Well, if this was there before, we should keep it. Could you add a sample to the patch, e.g. showing in an extra wxFrame in the treectrl sample how you'd implement the checkbox tree control? The patch can be separate, I'll commit the main one.

comment:4 Changed 6 years ago by malcompl

Maybe it would be better to add wxTR_CHECKBOXES flag and implement that possibility in the base class.

comment:5 Changed 6 years ago by vadz

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

This seems to have been already applied...

I do support the request to update the treectrl sample to show the new functionality, it would be really useful, TIA!

comment:6 Changed 6 years ago by malcompl

  • Resolution fixed deleted
  • Status changed from closed to reopened

Small docs corrections ;)

Ok, I'll try update the sample.

Changed 6 years ago by malcompl

comment:7 Changed 6 years ago by malcompl

In my last patch I forgot to update some crucial fragments of code, so I'm attaching a correction patch and updated the treectrl sample.

Changed 6 years ago by malcompl

Changed 6 years ago by malcompl

comment:8 Changed 6 years ago by roebling

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

Thanks, applied again.

comment:9 Changed 6 years ago by roebling

  • Resolution fixed deleted
  • Status changed from closed to reopened

Hm, the sample doesn't actually toggle the state icon (wxGTK).

comment:10 Changed 6 years ago by malcompl

Set Toggle alternate state images on Style menu and try to change state again, it works well. The problem isn't burried in TreeCtrl, it's probably wxRendererNative::DrawCheckBox() problem under wxGTK.

Small fixes for better apperance and functionality of editlabel in patch.

And samples compilation fix:

Index: samples/treectrl/treetest.cpp
===================================================================
--- samples/treectrl/treetest.cpp	(revision 54344)
+++ samples/treectrl/treetest.cpp	(working copy)
@@ -916,13 +916,13 @@
     }
     else
     {
-#if 0
+    #ifdef __WXWINDOWS__
         int width  = ::GetSystemMetrics(SM_CXMENUCHECK),
             height = ::GetSystemMetrics(SM_CYMENUCHECK);
-#else
+    #else
         int width = 16;
         int height = 16;
-#endif
+    #endif
 
         // make an state checkbox image list
         states = new wxImageList(width, height, true);

comment:11 Changed 6 years ago by malcompl

I found the problem, it is in wxImageList which uses wxBitmap copy ctor (copy constructor, uses reference counting), so there are 2 the same references to last draws bitmap (checked) in state image list.

I'm preparing to refactore wxImageList.

comment:12 Changed 6 years ago by vadz

  • Status changed from reopened to infoneeded

Malcom, which, if any, patches above should still be applied and which problems do they fix? Or should we wait for wxImageList refactoring (maybe it would be better to discuss it on wx-dev first?)?

I'm a bit lost in this patch so I'd appreciate any explanation about the current situation.

TIA!

comment:13 Changed 6 years ago by malcompl

  • Status changed from infoneeded to accepted

Ok, I updated and adapted to current situation patch's parts which should be applied.
The wxImageList problem is not affiliated with wxTreeCtrl, but it is visible here (in tree samples), so we should resolve this ticket.

comment:14 Changed 6 years ago by malcompl

Updated some parts of patch again.

Changed 6 years ago by malcompl

comment:15 Changed 6 years ago by vadz

  • Status changed from accepted to infoneeded

I've applied the patch to the sample, thanks. But I'm confused by the treedit.patch as it seems to have nothing to do with the state icons but rather changes something (what?) for the tree edit control. Could you please explain what does it do and possibly break it in 2 parts if there is a state item part in it too so that it could be applied independently of the text control fixes?

TIA!

comment:16 Changed 6 years ago by malcompl

  • Status changed from infoneeded to accepted

First I updated some part of code when we calculate/get size, offset and margin betwen state images and icon images. Next I added "support" state images to wxGenericTreeCtrl::GetBoundingRect and uses it in wxTreeTextCtrl ctor for gets position and size of item's text label to show edit control in correct position.

comment:17 Changed 6 years ago by malcompl

Small bug: if we turn to alternate state images and change some item's stae and back to normal state, state's image is broken when current state > normal state count. This path fixed that problem.

comment:18 Changed 6 years ago by malcompl

I updated some parts of patch that should been appiled.
Updated item calculate size and GetBoundingRect for support state images and clean some state images code.
Updated wxTreeTextCtrl ctor for support state images (correct size and position - get from GetBoundingRect).
Updated patch for fix broken state image problem.

Changed 6 years ago by malcompl

Changed 6 years ago by malcompl

Changed 6 years ago by malcompl

comment:19 Changed 6 years ago by vadz

  • Status changed from accepted to new

As treerect.patch, treeedit.patch and resetbroken.patch are new I'm resetting the bug into the new state too (but for the next patches a new item should really be opened as this one begins to become confusing).

comment:20 Changed 6 years ago by VZ

(In [58407]) corrections to size calculations for state images (see #9591)

comment:21 Changed 6 years ago by VZ

(In [58408]) use GetBoundingRect() for the in place text control positioning instead of duplicating its code (see #9591)

comment:22 Changed 6 years ago by VZ

(In [58409]) fix state images after changing their style (see #9591)

comment:23 Changed 6 years ago by vadz

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

Patches committed in the revisions mentioned above, the sample looks very nice now, thanks!

P.S. Please don't reopen this ticket any more but start a new one for any subsequent patches, TIA.

comment:24 Changed 18 months ago by VZ

(In [73824]) EVT_TREE_STATE_IMAGE_CLICK is not Windows-only any more.

State images in wxTreeCtrl are now supported under all platforms.

See #9591.

Note: See TracTickets for help on using tickets.