Skip to content

Reorganized monitor-bound utils in Tree #2154

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 16, 2025
Merged

Reorganized monitor-bound utils in Tree #2154

merged 1 commit into from
Jun 16, 2025

Conversation

@amartya4256
Copy link
Contributor

@amartya4256 amartya4256 commented May 14, 2025 •
edited
Loading

This commit contributes to moving
Tree:getContainingMonitorBoundsInPixels and
Tree:fitRectangleBoundsIntoMonitor to CoordinateSystemMapper and Display,
respectively for better clarity and encapsulation.

contributes to
#62 and
#128

@amartya4256 amartya4256 force-pushed the amartya4256/table_tooltip_fix branch 2 times, most recently from 03834e3 to e7b3a0c Compare May 14, 2025 16:05
@amartya4256 amartya4256 changed the title Amartya4256/table tooltip fix Fit Table Tooltip in a single monitor for Win32 May 14, 2025
@amartya4256 amartya4256 linked an issue May 14, 2025 that may be closed by this pull request
Tooltip of Table freezing on spanning across multiple monitors vi-eclipse/Eclipse-Platform#302
Closed
@github-actions
Copy link
Contributor

github-actions bot commented May 14, 2025 •
edited
Loading

Test Results

   545 files  ±0     545 suites  ±0   27m 46s ⏱️ +6s
 4 400 tests ±0   4 382 ✅ ±0   18 💤 ±0  0 ❌ ±0 
16 727 runs  ±0  16 587 ✅ ±0  140 💤 ±0  0 ❌ ±0 

Results for commit fd93049. ± Comparison against base commit 18eaec6.

♻️ This comment has been updated with latest results.

@amartya4256 amartya4256 force-pushed the amartya4256/table_tooltip_fix branch from e7b3a0c to 8411dc7 Compare May 16, 2025 14:46
@amartya4256 amartya4256 force-pushed the amartya4256/table_tooltip_fix branch 3 times, most recently from 2c65bce to 44c215e Compare June 16, 2025 14:03
@amartya4256 amartya4256 changed the title Fit Table Tooltip in a single monitor for Win32 Reorganized monitor-bound utils in Tree Jun 16, 2025
@amartya4256 amartya4256 force-pushed the amartya4256/table_tooltip_fix branch from 44c215e to 64fb36e Compare June 16, 2025 14:16
HeikoKlare
HeikoKlare reviewed Jun 16, 2025
View reviewed changes
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.

The overall change looks fine. In my understanding, this is not a pure refactoring but adds some additional functionality in MultiZoomCoordinateSystemMapper::getContainingMonitorBoundsInPixels(Point) for the case that the given point is not monitor-aware, right? But in my understanding that is rather a fallback as that path will currently not be taken as only the cursor location is passed to the method which, in case a multi-zoom mapper is used, will always be a monitor-aware point.

In addition to that, there seem to be some missing / lost null checks.

@amartya4256 amartya4256 force-pushed the amartya4256/table_tooltip_fix branch 2 times, most recently from ce55334 to f96c653 Compare June 16, 2025 15:25
@amartya4256
Reorganized monitor-bound utils in Tree eclipse-platform#62
fd93049 
This commit contributes to moving
Tree:getContainingMonitorBoundsInPixels and
Tree:fitRectangleBoundsIntoMonitor to CoordinateSystemMapper and Display,
respectively for better clarity and encapsulation.

contributes to
eclipse-platform#62 and
eclipse-platform#128
@amartya4256 amartya4256 force-pushed the amartya4256/table_tooltip_fix branch from f96c653 to fd93049 Compare June 16, 2025 15:30
HeikoKlare
HeikoKlare approved these changes Jun 16, 2025
View reviewed changes
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 good now, thank you! I also tested the changes and did not find any regressions. In particular, the tooltip alignment of trees to fit into the monitor still works as expected.

@HeikoKlare HeikoKlare merged commit 89a168f into eclipse-platform:master Jun 16, 2025
17 checks passed
@HeikoKlare HeikoKlare deleted the amartya4256/table_tooltip_fix branch June 16, 2025 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HeikoKlare HeikoKlare HeikoKlare approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tooltip of Table freezing on spanning across multiple monitors

2 participants

@amartya4256 @HeikoKlare