Skip to content

Always manage custom tooltip in Tree in Win32 #62 #2137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amartya4256
Copy link
Contributor

The PR contributes to the creation of custom tooltip handles in the Tree while creating items. It is a necessity to always have custom tooltips as it needs to be fit inside a single monitor since there can be freezing problems while using per-monitor awareness if the tooltip spans over multiple monitors.

contributes to
#62 and #127

The commit allows to always create custom tooltip handles in the
Tree while creating items. The is necessary to always fit the tooltips
inside a single monitor as there can be freezing problems while using
per-monitor awareness if the tooltip spans over multiple monitors.

contributes to
eclipse-platform#62 and
eclipse-platform#127
Copy link
Contributor

github-actions bot commented May 9, 2025

Test Results

   545 files  + 6     545 suites  +6   30m 14s ⏱️ +31s
 4 377 tests +37   4 359 ✅ +35   18 💤 +3  0 ❌  - 1 
16 647 runs  +37  16 506 ✅ +35  141 💤 +3  0 ❌  - 1 

Results for commit 9d3eb8a. ± Comparison against base commit 4df99ba.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a nice simplification of the tooltip handling. But I have to admit that I do not understand why it works. Without these changes, isCustomToolTip() never yields "false" for me, even for the problematic breakpoints view. Please see the detailed comments for that.

Comment on lines -3840 to -3842
boolean isCustomToolTip () {
return hooks (SWT.MeasureItem);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please explain why this is not relevant anymore (or why it was relevant before)? In my understanding, it validates whether a MeasureItem listener was registered, but I have to admit that I do not understand the reasons. What will be the effect in case there was a tree for which such a listener was not registered and thus tool tips were not custom? I set a breakpoint for the case the !isCustomToolTip() and triggered tool tips in several Trees, but it was never triggered.

@@ -2200,7 +2200,7 @@ void createItem (TreeItem item, long hParent, long hInsertAfter, long hItem) {
}
}
}

createItemToolTips ();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this additional creation here?

This has some effect that I do no understand: without this addition, I do not face any calls to isCustomToolTip() that yields "false", even for the breakpoints view the original issue is about.

String[] strings = item [0].strings;
if (strings != null) text = strings [index [0]];
}
//TEMPORARY CODE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to get temporary removed from > 15 years ago removed :-)

@@ -211,7 +211,7 @@ void _addListener (int eventType, Listener listener) {
case SWT.PaintItem: {
customDraw = true;
style |= SWT.DOUBLE_BUFFERED;
if (isCustomToolTip ()) createItemToolTips ();
createItemToolTips ();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are three (?) places where createItemToolTips() is called. Can't we do it once e.g. in createHandle or doesn't that work?

return new LRESULT (OS.CDRF_NOTIFYPOSTPAINT | OS.CDRF_NEWFONT);
}
break;
//TEMPORARY CODE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be interesting to understand what this does. If everything works fine, we could just remove that, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants