Skip to content
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

feat: allow setting marker type for MarkerGroups #5630

Merged
merged 6 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ace.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ export namespace Ace {
}

export class MarkerGroup {
constructor(session: EditSession);
constructor(session: EditSession, options?: {markerType?: "fullLine" | "line"});
setMarkers(markers: MarkerGroupItem[]): void;
getMarkerAtPosition(pos: Position): MarkerGroupItem;
}
Expand Down
24 changes: 19 additions & 5 deletions src/marker_group.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/**
* @typedef {import("./edit_session").EditSession} EditSession
* @typedef {{range: import("./range").Range, className: string}} MarkerGroupItem
* @typedef {import("../ace-internal").Ace.LayerConfig} LayerConfig
*/
/**
* @typedef {import("./layer/marker").Marker} Marker
Expand All @@ -15,8 +16,16 @@ Potential improvements:
class MarkerGroup {
/**
* @param {EditSession} session
* @param {{markerType: "fullLine" | "line" | undefined}} [options] Options controlling the behvaiour of the marker.
* User `markerType` to control how the markers which are part of this group will be rendered:
* - `undefined`: uses `text` type markers where only text characters within the range will be highlighted.
* - `fullLine`: will fully highlight all the rows within the range, including the characters before and after the range on the respective rows.
* - `line`: will fully highlight the lines within the range but will only cover the characters between the start and end of the range.
*/
constructor(session) {
constructor(session, options) {
if (options)
this.markerType = options.markerType;

this.markers = [];
/**@type {EditSession}*/
this.session = session;
Expand Down Expand Up @@ -59,7 +68,7 @@ class MarkerGroup {
* @param {any} html
* @param {Marker} markerLayer
* @param {EditSession} session
* @param {{ firstRow: any; lastRow: any; }} config
* @param {LayerConfig} config
*/
update(html, markerLayer, session, config) {
if (!this.markers || !this.markers.length)
Expand Down Expand Up @@ -103,10 +112,15 @@ class MarkerGroup {
continue;
}

if (screenRange.isMultiLine()) {
markerLayer.drawTextMarker(html, screenRange, marker.className, config);
if (this.markerType === "fullLine") {
markerLayer.drawFullLineMarker(html, screenRange, marker.className, config);
} else if (screenRange.isMultiLine()) {
if (this.markerType === "line")
markerLayer.drawMultiLineMarker(html, screenRange, marker.className, config);
else
markerLayer.drawTextMarker(html, screenRange, marker.className, config);
} else {
markerLayer.drawSingleLineMarker(html, screenRange, marker.className, config);
markerLayer.drawSingleLineMarker(html, screenRange, marker.className + " ace_start" + " ace_br15", config);
}
}
}
Expand Down
83 changes: 82 additions & 1 deletion src/marker_group_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ module.exports = {

next();
},
"test: show markers": function() {
"test: should show and update markers": function() {
editor.resize(true);
editor.renderer.$loop._flush();
var markerGroup = new MarkerGroup(session1);
Expand Down Expand Up @@ -65,6 +65,87 @@ module.exports = {
editor.renderer.$loop._flush();
assert.equal(editor.container.querySelectorAll(".m1").length, 0);
},
"test: should show markers of fullLine type": function() {
editor.resize(true);
editor.renderer.$loop._flush();
var markerGroup = new MarkerGroup(session1, {markerType: "fullLine"});

// this marker should be rendered as a full block across lines 1 and 2
markerGroup.setMarkers([{
range: new Range(0, 12, 1, 4),
className: "ace_tooltip-marker_test m"
}]);
assert.ok(markerGroup.getMarkerAtPosition({row: 1, column: 1}));
editor.renderer.$loop._flush();
assert.equal(editor.container.querySelectorAll(".m").length, 1);

// Get dimensions of the full line marker
var markerSize = editor.container.querySelector(".m").getBoundingClientRect();
var lineHeight = editor.renderer.lineHeight;

// Height should be two lines
assert.equal(markerSize.height, 2 * lineHeight);
// Should start at the beginning of the line
assert.equal(markerSize.left, 0);
// Shoud be as wide as the marker layer itself.
assert.equal(markerSize.width, editor.renderer.$markerBack.element.getBoundingClientRect().width);
},
"test: should show markers of line type": function() {
editor.resize(true);
editor.renderer.$loop._flush();
var markerGroup = new MarkerGroup(session1, {markerType: "line"});

// this marker should be rendered just covering the range, but extending to the edge of the editor on newlines
markerGroup.setMarkers([{
range: new Range(0, 12, 1, 4),
className: "ace_tooltip-marker_test m"
}]);
assert.ok(markerGroup.getMarkerAtPosition({row: 1, column: 1}));
editor.renderer.$loop._flush();
// Should render two separate markers for each row
assert.equal(editor.container.querySelectorAll(".m").length, 2);

// Get dimensions of the first line marker
var markerSize = editor.container.querySelectorAll(".m")[0].getBoundingClientRect();
var lineHeight = editor.renderer.lineHeight;
var characterWidth = editor.renderer.characterWidth;

// Height should be one lines
assert.equal(markerSize.height, lineHeight);
// Should start at the 13th character (including 4px offset)
assert.equal(markerSize.left, 12 * characterWidth + 4);
// Shoud be as wide as the marker layer - 12 characters and the offset.
assert.equal(markerSize.width, editor.renderer.$markerBack.element.getBoundingClientRect().width - 12 * characterWidth - 4);
},
"test: should default to markers of text type": function() {
editor.resize(true);
editor.renderer.$loop._flush();

// We don't set options.markerType, should default to text markers
var markerGroup = new MarkerGroup(session1);

// this marker should be rendered just covering the range
markerGroup.setMarkers([{
range: new Range(0, 12, 1, 4),
className: "ace_tooltip-marker_test m"
}]);
assert.ok(markerGroup.getMarkerAtPosition({row: 1, column: 1}));
editor.renderer.$loop._flush();
// Should render two separate markers for each row
assert.equal(editor.container.querySelectorAll(".m").length, 2);

// Get dimensions of the first line marker
var markerSize = editor.container.querySelectorAll(".m")[0].getBoundingClientRect();
var lineHeight = editor.renderer.lineHeight;
var characterWidth = editor.renderer.characterWidth;

// Height should be one lines
assert.equal(markerSize.height, lineHeight);
// Should start at the 13th character (including 4px offset)
assert.equal(markerSize.left, 12 * characterWidth + 4);
// Shoud be as wide as the remaining characters in the range on the first line.
assert.equal(markerSize.width, 6 * characterWidth);
},
tearDown: function() {
editor.destroy();
}
Expand Down
Loading