Skip to content

Conversation

mkslanc
Copy link
Contributor

@mkslanc mkslanc commented Sep 15, 2025

Issue #, if available:

Description of changes:

  • make gutter tooltip to work similar HoverTooltip

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Pull Request Checklist:

Copy link

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

github-actions bot commented Oct 6, 2025

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

github-actions bot commented Oct 6, 2025

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

github-actions bot commented Oct 7, 2025

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

1 similar comment
Copy link

github-actions bot commented Oct 7, 2025

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

@mkslanc mkslanc marked this pull request as ready for review October 7, 2025 14:10
@mkslanc mkslanc changed the title WIP: Gutter Hover tooltip Gutter Hover tooltip Oct 7, 2025
…tants, replace `const` with `var` for consistency, and adjust destroy logic.
Copy link

github-actions bot commented Oct 7, 2025

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

codecov bot commented Oct 7, 2025

Codecov Report

❌ Patch coverage is 94.82759% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.88%. Comparing base (8f32aab) to head (670086f).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/lib/dom.js 77.27% 5 Missing ⚠️
src/mouse/default_gutter_handler.js 95.45% 3 Missing ⚠️
src/keyboard/gutter_handler.js 80.00% 1 Missing ⚠️
src/keyboard/gutter_handler_test.js 98.00% 1 Missing ⚠️
src/mouse/default_gutter_handler_test.js 97.22% 1 Missing ⚠️
src/tooltip.js 97.87% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5872      +/-   ##
==========================================
- Coverage   87.89%   87.88%   -0.01%     
==========================================
  Files         622      622              
  Lines       46441    46478      +37     
  Branches     7609     7614       +5     
==========================================
+ Hits        40821    40849      +28     
- Misses       5620     5629       +9     
Flag Coverage Δ
unittests 87.88% <94.82%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

select?: () => void
$lastScroll?: { t: number, vx: number, vy: number, allowed: number }
selectEnd?: () => void
tooltip?: Ace.GutterTooltip
Copy link
Member

Choose a reason for hiding this comment

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

why do we need to add this as public property, don't we have it as annotationTooltip already?

assert.notOk(/ace_security_fold/.test(firstLineGutterElement.className));
assert.ok(/ace_warning_fold/.test(firstLineGutterElement.className));
},
"test: sets position correctly when tooltipFollowsMouse false" : function(done) {
Copy link
Member

Choose a reason for hiding this comment

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

Why this was removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was specifically to test tooltip mode when tooltipFollowsMouse is false; now we removed this option, so all other tests are checking quite the same.

editor.on("mousedown", this.hide);
editor.renderer.getMouseEventTarget().addEventListener("mouseout", this.onMouseOut, true);
var target = editor.renderer.getMouseEventTarget();
if (target && typeof target.removeEventListener === "function") {
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed?

src/tooltip.js Outdated
|| isMousePressed
|| this.isOutsideOfText(this.lastEvent)
) {
if (!this.range || !this.range.contains(pos.row, pos.column) || isMousePressed || this.isOutsideOfText(
Copy link
Member

Choose a reason for hiding this comment

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

please revert unrelated formatting changes like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

src/tooltip.js Outdated
* @param {{point:{pageX:number,pageY:number},rect?:{top:number,right:number,bottom:number,left:number,width:number,height:number}}} anchor
* @param {Range} [range]
*/
$setPosition(editor, anchor, range) {
Copy link
Member

Choose a reason for hiding this comment

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

this looks a bit wrong, point information is contained in rect, and shouldPlaceAbove does not need to be overridden in gutterTooltip.

Copy link

github-actions bot commented Oct 9, 2025

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

…ype definitions, and adjust related logic for consistency and clarity.
Copy link

github-actions bot commented Oct 9, 2025

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

github-actions bot commented Oct 9, 2025

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

Copy link

One of the public type files has been updated, plase make sure there are no backwards incompatible changes done in the PR.

@maria-raluca-st maria-raluca-st merged commit d1a93fc into ajaxorg:master Oct 17, 2025
3 of 4 checks passed
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