[SOLVED] Treeview: full-width background coloring of nodes ... easy?

[d7_2_laz]

I think the big question about 'HideSelection' is, how it really (and then: consistently) should behave.
a) Show no visual indication of the selected node at all if unfocused
or
b) Show the unfocused selected node in a more decent color; that's what actually with Non-Rowselect'ed nodes do (gray instead of blue).
Is this already decided?
The docs say: it "indicates if the current selection is hidden when the control loses focus".

If the answer is definitely: a), rhen of course the current behaviour of Non-Rowselect'ed nodes is a bug (as you mentioned).

And then a subsequent question comes up:
Should with HideSelection/False (and only with HideSelection/False) an unfocused state of a selected node be visually indicated (eg. gray instead of blue)?

Edit:
and: for HideSelection/True: could or should or should Not - in the moment when defocusing happens - an 'empty border' be drawn around a previously selected node, like in the image below?

[d7_2_laz]

I tried patches for both variants, but for now i'm closing this excursion with an experimental patch-diff (on 'main') for variant a.
In scope is theme-draw only.
Assumptions:
HideSelection/True:  in the moment when defocusing happens, an 'empty border' is drawn around a previously selected node (hmm ...).
HideSelection/False: when hovering over an unfocused tree, with RowSelect hovered nodes are shown in a different/decent color. Not (yet) when RowSelect is off.

[wp]

Definitely, your patch is a great improvement. But, I guess, I don't like the border-only selection, I looks very unusual, in particular regarding a themed control.

What is what the themed treeview should look like under any of these combinations? Even the Windows Explorer may not be valid for comparison.  Hard to decide...

I looked at Delph again to take it as a reference, and let me summarize the observations:

HideSelection (tree not focused)
- HideSelection = true: The selected node is completely hidden.
- HideSelection = false. The selected node is shown with gray background.
--> I think we should do it this way, too.

RowSelect
- As already noted, RowSelect does not seem to work in Delphi --> I would keep our solution.

HotTrack
- HideSelection = true: Hot-track color is always the pale blue, no matter whether the tree is focused or not.
- HideSelection = false: Hot-track color is gray when the tree is not focused.
In the earlier message i considered this to be inconsistent, but it looks more correct from another point of view: There is only one theme data value for the hottrack state (the pale blue, ttItemHot) - and this should be applied to all hot-tracked nodes, no matter whether the tree is focused or not. And there is one theme data value for the selected node of the unfocused tree (gray, ttItemSelectedNotFocus), and this should be applied only to the selected node and not be "abused" for hottracking. This interpretation additionally has the advantage of being delphi-compatible.

[d7_2_laz]

I'd agree to all points, especially to: "I don't like the border-only selection" -> Me neither, absolutely ...
But had not been sure yet if it should appear or not. And whether this little experiment could be of general interest at all.

(Personally i prefer un-themed, using customdraw. And avoid HideSelection whenever possible, because i don't like when i cannot recognize where a selection had been. So it's a more theoretical thing for me, but of course it should be consistent).

Technically, a simple reason for the border-only selection is "GetElementDetails(ttItemNormal)" -> found no better element amongst those TThemedTreeview's elements which might apply better here without border.
(Ah, just see, i'd simply need to skip the first If-branch here within procedure DrawNodeText, ok).

I assume, with "the selected node is completely hidden" you don't mean to have an empty line ... but rather that any additional visual indication of 'was selected' shouldn't appear.

There is only one theme data value for the hottrack state (the pale blue, ttItemHot) - and this should be applied to all hot-tracked nodes

Fully agree, cannot see any benefit to keep the colors different for hot-track focused/unfocused.

I'll adapt a bit later using these assumptions so far.

[wp]

I assume, with "the selected node is completely hidden" you don't mean to have an empty line ... but rather that any additional visual indication of 'was selected' shouldn't appear.

Hard to find the correct words... The node itself of course is present, but the selection is not shown. The selected node in this case cannot be distinguished from all the other nodes.

[d7_2_laz]

wp, what you'd think about this version?

Edit: it's thought for themeddraw. Non-themeddraw will be inspected later.

[wp]

Almost... But with RowSelect = false and HideSelection = true the selection is drawn in the same way for the focused or nonfocused case. It should be gray when the tree is not focused.

I rearranged to code a bit and now I think it is working as it should (as far as themed drawing is concerned).

The background drawing part of  DrawNodeText:

Code: Pascal  [Select][+][-]

1.   procedure DrawNodeText(IsSelected, IsHot: Boolean; NdRect: TRect; AText: String);
2.   var
3.     Details: TThemedElementDetails;
4.     DrawNormal: Boolean;
5.   begin
6.     if tvoThemedDraw in Options then
7.     begin
8.       // Themed drawing here
9.       DrawNormal := (not RowSelect) and IsSelected and (not Focused) and HideSelection;
10.       if (IsSelected or IsHot) and not DrawNormal then
11.       begin
12.         if (not Focused) and (not HideSelection) and (not IsHot) then
13.           Details := ThemeServices.GetElementDetails(ttItemSelectedNotFocus)
14.         else
15.         if IsSelected then
16.           Details := ThemeServices.GetElementDetails(ttItemSelected)
17.         else
18.           Details := ThemeServices.GetElementDetails(ttItemHot);
19.         if not RowSelect then
20.           ThemeServices.DrawElement(Canvas.Handle, Details, NdRect, nil);
21.       end else
22.         Details := ThemeServices.GetElementDetails(ttItemNormal);
23.     end else
24.     // Non-themed drawing here
25.     if not RowSelect and (Canvas.Brush.Color <> clNone) then
26.       Canvas.FillRect(NdRect);   // colors not yet correct. Preparation of the default canvas must occur before firing the events.
27.   [...]

[wp]

Turning off the selection completely when the tree is not focused has an ugly side-effect: In the object inspector's object tree (which is a TTreeView, of course) the selected control is no longer highlighted when the user works in the form designer... Of course, this can be fixed by setting HideSelection to false, but users will have similar experiences in their own projects...

[d7_2_laz]

But with RowSelect = false and HideSelection = true the selection is drawn in the same way for the focused or nonfocused case. It should be gray when the tree is not focused

Oh sorry, yes, that had been dropped again on the last meters, because i'd struggled with a possible impact on the non-themed part lately.

Could you show your whole DrawNodeText procedure (the structure seems to be overall a bit different)?
Maybe as a short txt-file if it should be too long here.

But, there are so many traps inside this area - and you just told another one - that i'm asking myself if it 's really useful to pursue this HideSelection-change for themeddraw at all.

Btw, by this occasion i noticed the first time (i normally don't use HideSelecton/true or themeddraw) that obviously for nonthemed / noncustomdrawn the HideSelection did not work at all (i had tried with an original 3.8 treeview.inc and this seemed to confirm this). Can this really be the case? I'm surprised. Mined terrain .... maybe better leave it (personally i'd be fully happy with the customdraw imprevements).

[wp]

Yes, I rearranged the lines in DrawNodeText to keep themed and non-themed drawing apart, the old arrangement just blew my mind...

Here are the three complete drawing procedures of DoPaintNode which seem to work correctly:

Code: Pascal  [Select][+][-]

1.   { Draws the default normal node background }
2.   procedure DrawNormalBackground(ARect: TRect);
3.   begin
4.     if Canvas.Brush.Color <> clNone then
5.       Canvas.FillRect(ARect);
6.   end;
7.  
8.   { Default-draws the background of selected and hot-tracked nodes over the full
9.     client width. This does not occur when the tree is not in RowSelect mode,
10.     or when the preceding OnAdvancedCustomDrawItem event handler has been
11.     exited with DefaultDraw = fals. }
12.   procedure DrawSpecialBackground(IsSelected, IsHot: Boolean; ARect: TRect);
13.   var
14.     Details: TThemedElementDetails;
15.     DrawNormal: Boolean;
16.   begin
17.     if not RowSelect then
18.       exit;
19.     if not (IsSelected or IsHot) then
20.       exit;
21.     DrawNormal := IsSelected and not (Focused or IsHot) and HideSelection;
22.     if (tvoThemedDraw in Options) and (not DrawNormal) then
23.     begin
24.       if (tvoFocusedPainting in FStates) then
25.       begin
26.         if (IsSelected and (not Focused) and not HideSelection) then
27.           Details := ThemeServices.GetElementDetails(ttItemSelectedNotFocus)
28.         else
29.         if IsHot then
30.           Details := ThemeServices.GetElementDetails(ttItemHot)
31.         else
32.         if IsSelected then
33.           Details := ThemeServices.GetElementDetails(ttItemSelected);
34.         ThemeServices.DrawElement(Canvas.Handle, Details, ARect, nil);
35.       end;
36.     end else
37.     if (IsSelected or IsHot) and (Canvas.Brush.Color <> clNone) then
38.       Canvas.FillRect(ARect);
39.   end;
40.  
41.   { Draws first the node text background unless deactivated by user selection
42.     in the custom-draw events. Then the node text is drawn. }
43.   procedure DrawNodeText(IsSelected, IsHot: Boolean; NdRect: TRect; AText: String);
44.   var
45.     Details: TThemedElementDetails;
46.     DrawNormal: Boolean;
47.   begin
48.     if tvoThemedDraw in Options then
49.     begin
50.       // Themed drawing here
51.       DrawNormal := not (RowSelect or Focused or IsHot) and IsSelected and HideSelection;
52.       if (IsSelected or IsHot) and not DrawNormal then
53.       begin
54.         if (not Focused) and (not HideSelection) and (not IsHot) then
55.           Details := ThemeServices.GetElementDetails(ttItemSelectedNotFocus)
56.         else
57.         if IsHot then
58.           Details := ThemeServices.GetElementDetails(ttItemHot)
59.         else
60.         if IsSelected then
61.           Details := ThemeServices.GetElementDetails(ttItemSelected);
62.         if not RowSelect then
63.           ThemeServices.DrawElement(Canvas.Handle, Details, NdRect, nil);
64.       end else
65.         Details := ThemeServices.GetElementDetails(ttItemNormal);
66.     end else
67.     // Non-themed drawing of text background here
68.     if not RowSelect and (Canvas.Brush.Color <> clNone) then
69.       Canvas.FillRect(NdRect);
70.  
71.     // Draw the node text
72.     NdRect.Offset(ScaleX(2, 96), 0);
73.     if (tvoThemedDraw in Options) then
74.     begin
75.       if not (Enabled and Node.Enabled) then
76.         Details.State := 4; // TmSchema.TREIS_DISABLED = 4
77.       ThemeServices.DrawText(Canvas, Details, AText, NdRect, DT_LEFT or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX, 0);
78.     end
79.     else
80.     begin
81.       SetBkMode(Canvas.Handle, TRANSPARENT);
82.       DrawText(Canvas.Handle, PChar(AText), -1, NdRect, DT_LEFT or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX);
83.     end;
84.   end;

and these are the first few lines of DoPaint which incorporates your addition:

Code: Pascal  [Select][+][-]

1. procedure TCustomTreeView.DoPaint;
2. var
3.   a,HalfBorderWidth:integer;
4.   SpaceRect, DrawRect: TRect;
5.   Node: TTreeNode;
6.   InsertMarkRect: TRect;
7.   bkColor: TColor;
8. begin
9.   if [tvsPainting] * FStates <> [] then Exit;
10.   Include(FStates, tvsPainting);
11.   try
12.     if Focused then
13.       Include(FStates,tvoFocusedPainting)
14.     else
15.     if (Selected <> nil) then
16.       Include(FStates,tvoFocusedPainting)
17.     else
18.       Exclude(FStates,tvoFocusedPainting);
19.     if (tvoAutoItemHeight in fOptions) then
20.       UpdateDefaultItemHeight;
21.     //UpdateScrollbars;
22.     with Canvas do
23.     begin
24. [...]

But, there are so many traps inside this area - and you just told another one - that i'm asking myself if it 's really useful to pursue this HideSelection-change for themeddraw at all.

I am in doubt myself, but I do not yet want to give up: What if we add a compatibility property such as HideSelectionMode = (hsmLaz, hsmDelphi) - hsmLaz (default) would keep the old Lazarus behaviour (HideSelection ignored) while hsmDelphi would activate the current state of the patch where the selection of the non-focused tree is hidden. Later we could decide whether to deprecate this in favour of the delphi-compatible mode giving enough time for users to change the HideSelection itself to false (which has the same effect as HideSelection = true(default) and HideSelectionMode = hsmLaz)

Btw, by this occasion i noticed the first time (i normally don't use HideSelecton/true or themeddraw) that obviously for nonthemed / noncustomdrawn the HideSelection did not work at all (i had tried with an original 3.8 treeview.inc and this seemed to confirm this). Can this really be the case? I'm surprised.

That's my impression, too. I just uploaded a new version of the Treeviews customdrawin example (in the examples/treeview folder) extended by a HideSelection option. And when I compile this with an older version it really looks as if Lazarus had never supported HideSelection for the non-themed tree.

[d7_2_laz]

Yes, with this version of DrawNodeText it looks - 'themed' in scope - again like here yesterday evening (until something went wrong, whilst i struggled with a side-effect on non-themed elements. At that point i asked myself, is it worthy, i never got aware about complaints on HideSelection).

I'd understood the intention of the rearrangement in DrawNodeText, but wasn't sure what would follow. And yes, the diverse queries for 'if tvoThemedDraw', or if not, mixed into multiple code lines felt very confusing. It's better now.

Such compatibility property ('HideSelectionMode ') as you describe could be a viable path. As it should hinder diverse regression effects, ie. in the Designer, or for user apps.

Had tested with your themed_hottrack-rowselect_hideselection.zip, and with my own demo with those various style flavours too (with special focus, for to go sure, that results of custom-draw are not damaged). Had tested with a real app too -> ok.

For the non-themed and non customdrawing way there's of course no change yet regarding HideSelection, but it would be part of this default layer.

[wp]

Just committed the final (?...) version implementing HideSelection in a Delphi-compatible way for both themed and non-themed painting. There is a flag HideSelectionMode = (hsmLaz, hsmDelphi) to switch between Lazarus (HideSelection is ignored) and Delphi modes (HideSelection really hides the selection of the non-focussed tree). A function SelectionIsHidden() is provided to tell whether the Selection should be drawn or not. This is important for custom painting.

I was able to refactor the code quite a bit because I noticed that the background and text drawing sub-procedures use the same ThemedElement settings although this was well-hidden among the many if conditions. Now I introduced a lookup table for the ThemedElement selection depending on Focused, Selected and Hot states.

Please see the demo project in the examples/treeview folder.

[d7_2_laz]

I noticed that the background and text drawing sub-procedures use the same ThemedElement settings although this was well-hidden among the many if conditions.

Yes, that had me make wondering too.

After retrieving 'main' yesterday evening (= after the change had been committed) i tried the example, but with regard to the new properties i stumbled across an if switch that kept them still deactive.
Instead of many words a picture (what might be going on here?):

[wp]

what might be going on here?

Nothing special. I just want to be able to use the customdrawing test project also with older Lazarus versions to find out differences, old behaviour, or regressions. But the HideSelectionMode property is available only in v4.99 (main) or later. In order to be able to compile the project also with older versions, I put the related code in a version check directive; of course the new drawing features are not available then.

In your case - I think you use v3.8 which you patch manually - the situation is more complex because the version check fails but the propery (and all the related code) is available nevertheless. Therefore: simply remove the {$IF...} and {$ENDIF}.

[d7_2_laz]

In your case - I think you use v3.8 which you patch manually - the situation is more complex because the version check fails but the propery (and all the related code) is available nevertheless.

Nonono, 'main' with having the changes (and so this recent example too).
(and no dblclick onto the project's lpi for to start 3.8 behind the scenes, but by explicitely starting lazarus.exe from 'main' and opening the new project then). 4.99 s. screenshot, and LCL_FullVersion 499000 as a MessageBox in the FormCreate does tell me.

Therefore: simply remove the {$IF...} and {$ENDIF}.

Thought directly about to proceed so, but ... there are other LCL_FullVersion 'if's too, and i'd rather prefer to understand why than to step 'blindly'. My second thought was, might it depend on a newer FPC version (my make_laz.bat's path points to the 3.2.2 fpc bin), but i don't really believe. Which fpc version did you use? if you should use 3.2.2 too, then it's rather something silly here on my desk ...