feat: summary of annotations in folded lines #5117

akoreman · 2023-04-06T11:30:38Z

Currently, when there are gutter annotations for lines which are folded away, the gutter annotations are invisible. This change adds a (optional) one-line summary of the annotations in the folded code:

Added as an option to not break existing users who have custom gutter annotation icons in their themes (they would see their custom icons for 'normal' annotations and the default icons for annotations in folds).

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

codecov · 2023-04-06T11:33:33Z

Codecov Report

Patch coverage: 98.11% and project coverage change: +0.03 🎉

Comparison is base (3a3849b) 86.79% compared to head (3b180cb) 86.83%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5117      +/-   ##
==========================================
+ Coverage   86.79%   86.83%   +0.03%     
==========================================
  Files         558      558              
  Lines       43611    43718     +107     
  Branches     6780     6794      +14     
==========================================
+ Hits        37853    37962     +109     
+ Misses       5758     5756       -2

Flag	Coverage Δ
unittests	`86.83% <98.11%> (+0.03%)`	⬆️

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

Impacted Files	Coverage Δ
src/css/editor.css.js	`100.00% <ø> (ø)`
src/editor.js	`79.85% <ø> (ø)`
src/mouse/default_gutter_handler.js	`69.62% <93.33%> (+5.28%)`	⬆️
src/layer/gutter.js	`89.77% <100.00%> (+1.00%)`	⬆️
src/mouse/default_gutter_handler_test.js	`99.39% <100.00%> (+0.40%)`	⬆️
src/virtual_renderer.js	`77.90% <100.00%> (+0.04%)`	⬆️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

nightwing · 2023-04-06T11:38:02Z

The change of anchor position was accidental in the last pr, but maybe we should keep it aligned to the icon? I think the first one looks better, and it is worth to consider keeping it as is.

akoreman · 2023-04-06T11:57:44Z

The main problem I noticed was the inconsistency between the anchoring for SVG and PNG icon gutter tooltips so I changed it to be the old/second behavior for both. But I'm also okay with changing it so we will have the first/new behavior for both, agree that the first/new one looks a bit more coherent.

nightwing · 2023-04-07T08:33:32Z

src/css/editor.css.js

    background-image: url("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAMAAAAoLQ9TAAABOFBMVEX/////////QRswFAb/Ui4wFAYwFAYwFAaWGAfDRymzOSH/PxswFAb/SiUwFAYwFAbUPRvjQiDllog5HhHdRybsTi3/Tyv9Tir+Syj/UC3////XurebMBIwFAb/RSHbPx/gUzfdwL3kzMivKBAwFAbbvbnhPx66NhowFAYwFAaZJg8wFAaxKBDZurf/RB6mMxb/SCMwFAYwFAbxQB3+RB4wFAb/Qhy4Oh+4QifbNRcwFAYwFAYwFAb/QRzdNhgwFAYwFAbav7v/Uy7oaE68MBK5LxLewr/r2NXewLswFAaxJw4wFAbkPRy2PyYwFAaxKhLm1tMwFAazPiQwFAaUGAb/QBrfOx3bvrv/VC/maE4wFAbRPBq6MRO8Qynew8Dp2tjfwb0wFAbx6eju5+by6uns4uH9/f36+vr/GkHjAAAAYnRSTlMAGt+64rnWu/bo8eAA4InH3+DwoN7j4eLi4xP99Nfg4+b+/u9B/eDs1MD1mO7+4PHg2MXa347g7vDizMLN4eG+Pv7i5evs/v79yu7S3/DV7/498Yv24eH+4ufQ3Ozu/v7+y13sRqwAAADLSURBVHjaZc/XDsFgGIBhtDrshlitmk2IrbHFqL2pvXf/+78DPokj7+Fz9qpU/9UXJIlhmPaTaQ6QPaz0mm+5gwkgovcV6GZzd5JtCQwgsxoHOvJO15kleRLAnMgHFIESUEPmawB9ngmelTtipwwfASilxOLyiV5UVUyVAfbG0cCPHig+GBkzAENHS0AstVF6bacZIOzgLmxsHbt2OecNgJC83JERmePUYq8ARGkJx6XtFsdddBQgZE2nPR6CICZhawjA4Fb/chv+399kfR+MMMDGOQAAAABJRU5ErkJggg==");
    background-repeat: no-repeat;
    background-position: 2px center;
 }

-.ace_gutter-cell.ace_warning, .ace_icon.ace_warning {
+.ace_gutter-cell.ace_warning, .ace_gutter-tooltip .ace_icon.ace_warning {


Why is this change needed? We may want to use the icon in other places too, e.g. in documentation tooltip.

nightwing · 2023-04-07T08:40:54Z

src/css/editor.css.js

+    float: left;
+}
+
+.ace_gutter-cell.ace_error, .ace_gutter-tooltip .ace_icon.ace_error {


I think this change will break cloud9, because there we use custom gutter cell backgrounds for errors and breakpoints. With this both will be visible.

If the goal is simply to have a node for getting client rect, maybe we can use client rect from element and offset by editor.renderer.$gutterLayer.$padding.left?

Let's try to not break c9 🙂 could you explain how this change could affect c9?

I assume they use selectors like .ace_gutter-cell.ace_error for their custom backgrounds and nothing changes for these selectors. The only change here is that the background set for .ace_icon.ace_error is now scoped to .ace_gutter-tooltip (to avoid the background image showing up twice).

My bad, i thought this changes ace to set background on ace_icon instead of ace_gutter-cell, which would break the css override in cloud9.

I still think using the value from padding instead of adding a completely empty element would be better, but this may be ok too.

akoreman · 2023-04-10T13:56:43Z

Refactored and simplified the PR a bit.

akoreman · 2023-04-14T21:48:29Z

I'm working on incorporating this PR into a slightly larger improvement of the gutter tooltip. Setting PR to draft for now.

InspiredGuy

looks good, but left some comments: mostly about code style and nitpicks

src/ext/options.js

src/layer/gutter.js

src/mouse/default_gutter_handler.js

nightwing · 2023-04-19T12:04:52Z

src/mouse/default_gutter_handler.js

+        var annotation;
+
+        if (annotationsInRow)
+            annotation = {text: [...annotationsInRow.text], type: [...annotationsInRow.type]};


let's not use code constructs that cannot be efficiently compiled for old browsers, also why do we need to copy the array here?

I want to add an element to the array used to construct the tooltip so I want to copy the array by value. Switched to a different implementation for that to not use spread.

src/mouse/default_gutter_handler.js

akoreman added 2 commits April 6, 2023 13:25

fix: gutter tooltip anchoring

67e7fcc

add semicolon

8109ed8

akoreman marked this pull request as draft April 6, 2023 12:53

akoreman and others added 3 commits April 6, 2023 17:04

Change anchor point to be on icon

89841a5

set tooltipFollowsMouse back to true by default

33facc4

small css change

9acd13c

akoreman marked this pull request as ready for review April 6, 2023 18:02

whitespace fix

598ca4a

nightwing reviewed Apr 7, 2023

View reviewed changes

akoreman and others added 2 commits April 8, 2023 12:38

Merge branch 'ajaxorg:master' into gutter_tooltip_fix

68bb9d7

Refactoring

3190131

akoreman and others added 4 commits April 12, 2023 13:13

Merge branch 'ajaxorg:master' into gutter_tooltip_fix

e59b8d8

Merge branch 'ajaxorg:master' into gutter_tooltip_fix

a579507

Merge branch 'ajaxorg:master' into gutter_tooltip_fix

fafb66d

add summary of annotations in folded code

b308c0a

akoreman marked this pull request as draft April 14, 2023 21:43

akoreman changed the title ~~fix: gutter tooltip anchoring~~ feat: summary of annotations in folded lines Apr 14, 2023

akoreman and others added 7 commits April 15, 2023 11:55

add SVG icons for lines with folded annotations

27d6197

fix icon positioning

0153067

fix failing tests

4e8a3c0

add test

4a4ca09

tweak new tests

78b2f81

add annotations in fold as optional

3a26660

add tooltip check to tests

448fd2c

refactor and cleanup

095bf2b

akoreman marked this pull request as ready for review April 18, 2023 20:02

InspiredGuy requested a review from nightwing April 19, 2023 09:54

InspiredGuy reviewed Apr 19, 2023

View reviewed changes

refactoring

9dcf632

nightwing reviewed Apr 19, 2023

View reviewed changes

akoreman added 7 commits April 19, 2023 14:22

Change name kitchen-sink option

6083beb

make svg viewbox size consistent

a4a2652

test tweak

15bd2d0

Merge branch 'ajaxorg:master' into gutter_tooltip_fix

39fb5f4

change array copy to use Array.from

0c0727b

test fix

99f9d36

tweak

3b180cb

InspiredGuy requested a review from nightwing April 20, 2023 14:55

InspiredGuy approved these changes Apr 20, 2023

View reviewed changes

akoreman merged commit dc63ba9 into ajaxorg:master Apr 21, 2023

akoreman deleted the gutter_tooltip_fix branch April 21, 2023 08:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: summary of annotations in folded lines #5117

feat: summary of annotations in folded lines #5117

akoreman commented Apr 6, 2023 •

edited

Loading

codecov bot commented Apr 6, 2023 •

edited

Loading

nightwing commented Apr 6, 2023

akoreman commented Apr 6, 2023 •

edited

Loading

nightwing Apr 7, 2023

nightwing Apr 7, 2023

akoreman Apr 8, 2023

nightwing Apr 14, 2023

akoreman commented Apr 10, 2023

akoreman commented Apr 14, 2023 •

edited

Loading

InspiredGuy left a comment

nightwing Apr 19, 2023

akoreman Apr 20, 2023

feat: summary of annotations in folded lines #5117

feat: summary of annotations in folded lines #5117

Conversation

akoreman commented Apr 6, 2023 • edited Loading

codecov bot commented Apr 6, 2023 • edited Loading

Codecov Report

nightwing commented Apr 6, 2023

akoreman commented Apr 6, 2023 • edited Loading

nightwing Apr 7, 2023

Choose a reason for hiding this comment

nightwing Apr 7, 2023

Choose a reason for hiding this comment

akoreman Apr 8, 2023

Choose a reason for hiding this comment

nightwing Apr 14, 2023

Choose a reason for hiding this comment

akoreman commented Apr 10, 2023

akoreman commented Apr 14, 2023 • edited Loading

InspiredGuy left a comment

Choose a reason for hiding this comment

nightwing Apr 19, 2023

Choose a reason for hiding this comment

akoreman Apr 20, 2023

Choose a reason for hiding this comment

akoreman commented Apr 6, 2023 •

edited

Loading

codecov bot commented Apr 6, 2023 •

edited

Loading

akoreman commented Apr 6, 2023 •

edited

Loading

akoreman commented Apr 14, 2023 •

edited

Loading