From 26a9eb557f0bad298fff99bdde51f3cb391e25c4 Mon Sep 17 00:00:00 2001 From: abose Date: Sun, 16 Feb 2025 15:11:50 +0530 Subject: [PATCH 1/8] chore: merge Overlapping Scrollbar Tickmarks for Efficiency Refactored _renderMarks(posArray) to merge overlapping or adjacent tickmarks, reducing DOM elements while maintaining accuracy. Instead of rendering multiple 2px markers, contiguous marks are merged into a single div. Adjusted styles (brackets.less): .tickmark height is now based on 2px bg color, added .tickmark-side (5px) for better side markers in scrollbar instead of line markers --- src/search/ScrollTrackMarkers.js | 80 ++++++++++++++++++++++++++------ src/styles/brackets.less | 16 +++++-- 2 files changed, 76 insertions(+), 20 deletions(-) diff --git a/src/search/ScrollTrackMarkers.js b/src/search/ScrollTrackMarkers.js index 83ccce6bd6..2c5a72e644 100644 --- a/src/search/ScrollTrackMarkers.js +++ b/src/search/ScrollTrackMarkers.js @@ -104,9 +104,9 @@ define(function (require, exports, module) { } - function _getScrollbar(editor) { + function _getScrollbar(editorInstance) { // Be sure to select only the direct descendant, not also elements within nested inline editors - return $(editor.getRootElement()).children(".CodeMirror-vscrollbar"); + return $(editorInstance.getRootElement()).children(".CodeMirror-vscrollbar"); } /** @@ -123,29 +123,47 @@ define(function (require, exports, module) { trackHt -= trackOffset * 2; } else { // No scrollbar: use the height of the entire code content - var codeContainer = $(editor.getRootElement()).find("> .CodeMirror-scroll > .CodeMirror-sizer > div > .CodeMirror-lines > div")[0]; + var codeContainer = $(editor.getRootElement()) + .find("> .CodeMirror-scroll > .CodeMirror-sizer > div > .CodeMirror-lines > div")[0]; trackHt = codeContainer.offsetHeight; trackOffset = codeContainer.offsetTop; } } + function _getTop(editorInstance, pos) { + let cm = editorInstance._codeMirror, + editorHt = cm.getScrollerElement().scrollHeight, + cursorTop; + + let wrapping = cm.getOption("lineWrapping"), + singleLineH = wrapping && cm.defaultTextHeight() * 1.5; + let curLine = pos.line; + let curLineObj = cm.getLineHandle(curLine); + if (wrapping && curLineObj.height > singleLineH) { + cursorTop = cm.charCoords(pos, "local").top; + } else { + cursorTop = cm.heightAtLine(curLineObj, "local"); + } + // return top + return (Math.round(cursorTop / editorHt * trackHt) + trackOffset) - 1; // 1px Centering correction + } + + const MARKER_HEIGHT = 2; + /** - * Add all the given tickmarks to the DOM in a batch + * Add merged tickmarks to the scrollbar track * @private */ function _renderMarks(posArray) { - var html = "", - cm = editor._codeMirror, + let cm = editor._codeMirror, editorHt = cm.getScrollerElement().scrollHeight; - // We've pretty much taken these vars and the getY function from CodeMirror's annotatescrollbar addon - // https://github.com/codemirror/CodeMirror/blob/master/addon/scroll/annotatescrollbar.js - var wrapping = cm.getOption("lineWrapping"), + let wrapping = cm.getOption("lineWrapping"), singleLineH = wrapping && cm.defaultTextHeight() * 1.5, curLine = null, curLineObj = null; - function getY(cm, pos) { + function getY(pos) { if (curLine !== pos.line) { curLine = pos.line; curLineObj = cm.getLineHandle(curLine); @@ -156,13 +174,41 @@ define(function (require, exports, module) { return cm.heightAtLine(curLineObj, "local"); } + var markPositions = []; + + // Convert line positions to scrollbar positions posArray.forEach(function (pos) { - var cursorTop = getY(cm, pos), + var cursorTop = getY(pos), top = Math.round(cursorTop / editorHt * trackHt) + trackOffset; - top--; // subtract ~1/2 the ht of a tickmark to center it on ideal pos + top--; // Centering correction + markPositions.push({ top: top, bottom: top + MARKER_HEIGHT }); // Default height is 2px + }); - html += "
"; + // Sort positions by top coordinate + markPositions.sort((a, b) => a.top - b.top); + + // Merge overlapping or adjacent tickmarks + var mergedMarks = []; + markPositions.forEach(({ top, bottom }) => { + if (mergedMarks.length > 0) { + let lastMark = mergedMarks[mergedMarks.length - 1]; + + if (top <= lastMark.bottom + 1) { + // Extend the existing mark + lastMark.bottom = Math.max(lastMark.bottom, bottom); + lastMark.height = lastMark.bottom - lastMark.top; // Adjust height + return; + } + } + // Create a new mark + mergedMarks.push({ top, bottom, height: bottom - top }); }); + + // Generate and insert tickmarks into the DOM + var html = mergedMarks.map(({ top, height }) => + `
` + ).join(""); + $(".tickmark-track", editor.getRootElement()).append($(html)); } @@ -239,11 +285,15 @@ define(function (require, exports, module) { function markCurrent(index) { // Remove previous highlight first if ($markedTickmark) { - $markedTickmark.removeClass("tickmark-current"); + $markedTickmark.remove(); $markedTickmark = null; } if (index !== -1) { - $markedTickmark = $(".tickmark-track > .tickmark", editor.getRootElement()).eq(index).addClass("tickmark-current"); + const parkPos = marks[index]; + const top = _getTop(editor, parkPos); + $markedTickmark = $( + `
`); + $(".tickmark-track ").append($markedTickmark); } } diff --git a/src/styles/brackets.less b/src/styles/brackets.less index bb44649381..d3a4e02d18 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -2621,18 +2621,24 @@ textarea.exclusions-editor { .tickmark { position: absolute; width: 12px; - height: 1px; + height: 2px; background-color: #eddd23; - border-top: 1px solid #e0d123; - border-bottom: 1px solid #d4c620; + opacity: 0.85; // allow thumb to show through &.tickmark-current { background-color: #ed9823; - border-top: 1px solid #dd9128; - border-bottom: 1px solid #cb8320; z-index: 1; // ensure this one appears above overlapping sibling highlights } } + + .tickmark-side { + position: absolute; + width: 12px; + height: 5px; + border-left: 2px solid; + border-left-color: #e0d123; + opacity: 0.85; // allow thumb to show through + } } From 010462aca0e69b268859b4aa54df2387fa654f94 Mon Sep 17 00:00:00 2001 From: abose Date: Sun, 16 Feb 2025 16:46:24 +0530 Subject: [PATCH 2/8] refactor: scroll track markers so that multiple editors can have their own markers simultaneously --- src/search/FindReplace.js | 30 +-- src/search/ScrollTrackMarkers.js | 415 +++++++++++++++++-------------- src/styles/brackets.less | 1 + 3 files changed, 251 insertions(+), 195 deletions(-) diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js index cc79b14b0e..9d8c9e8bf6 100644 --- a/src/search/FindReplace.js +++ b/src/search/FindReplace.js @@ -393,10 +393,10 @@ define(function (require, exports, module) { } /** Removes the current-match highlight, leaving all matches marked in the generic highlight style */ - function clearCurrentMatchHighlight(cm, state) { + function clearCurrentMatchHighlight(editor, state) { if (state.markedCurrent) { state.markedCurrent.clear(); - ScrollTrackMarkers.markCurrent(-1); + ScrollTrackMarkers.markCurrent(-1, editor); } } @@ -405,7 +405,7 @@ define(function (require, exports, module) { const pos = editor.getCursorPos(false, "start"); cm.operation(function () { var state = getSearchState(cm); - clearCurrentMatchHighlight(cm, state); + clearCurrentMatchHighlight(editor, state); let curIndex = editor.indexFromPos(pos); curIndex = curIndex >= 1 ? curIndex-- : 0; @@ -424,7 +424,7 @@ define(function (require, exports, module) { {from: thisMatch.start, to: thisMatch.end}, false); // Update current-tickmark indicator - only if highlighting enabled (disabled if FIND_HIGHLIGHT_MAX threshold hit) if (state.marked.length) { - ScrollTrackMarkers.markCurrent(state.matchIndex); // _updateFindBarWithMatchInfo() has updated this index + ScrollTrackMarkers.markCurrent(state.matchIndex, editor); // _updateFindBarWithMatchInfo() has updated this index } } @@ -454,7 +454,7 @@ define(function (require, exports, module) { var cm = editor._codeMirror; cm.operation(function () { var state = getSearchState(cm); - clearCurrentMatchHighlight(cm, state); + clearCurrentMatchHighlight(editor, state); var nextMatch = _getNextMatch(editor, searchBackwards, pos); if (nextMatch) { @@ -464,7 +464,7 @@ define(function (require, exports, module) { {from: nextMatch.start, to: nextMatch.end}, searchBackwards); // Update current-tickmark indicator - only if highlighting enabled (disabled if FIND_HIGHLIGHT_MAX threshold hit) if (state.marked.length) { - ScrollTrackMarkers.markCurrent(state.matchIndex); // _updateFindBarWithMatchInfo() has updated this index + ScrollTrackMarkers.markCurrent(state.matchIndex, editor); // _updateFindBarWithMatchInfo() has updated this index } } @@ -485,23 +485,25 @@ define(function (require, exports, module) { } /** Clears all match highlights, including the current match */ - function clearHighlights(cm, state) { + function clearHighlights(editor, state) { + const cm = editor._codeMirror; cm.operation(function () { state.marked.forEach(function (markedRange) { markedRange.clear(); }); - clearCurrentMatchHighlight(cm, state); + clearCurrentMatchHighlight(editor, state); }); state.marked.length = 0; state.markedCurrent = null; - ScrollTrackMarkers.clear(); + ScrollTrackMarkers.clear(editor); state.resultSet = []; state.matchIndex = -1; } - function clearSearch(cm) { + function clearSearch(editor) { + const cm = editor._codeMirror; cm.operation(function () { var state = getSearchState(cm); if (!state.parsedQuery) { @@ -509,7 +511,7 @@ define(function (require, exports, module) { } setQueryInfo(state, null); - clearHighlights(cm, state); + clearHighlights(editor, state); }); } @@ -544,7 +546,7 @@ define(function (require, exports, module) { cm.operation(function () { // Clear old highlights if (state.marked) { - clearHighlights(cm, state); + clearHighlights(editor, state); } if (!state.parsedQuery) { @@ -676,7 +678,7 @@ define(function (require, exports, module) { .on("close.FindReplace", function (e) { editor.lastParsedQuery = state.parsedQuery; // Clear highlights but leave search state in place so Find Next/Previous work after closing - clearHighlights(cm, state); + clearHighlights(editor, state); // Dispose highlighting UI (important to restore normal selection color as soon as focus goes back to the editor) toggleHighlighting(editor, false); @@ -770,7 +772,7 @@ define(function (require, exports, module) { if (editor) { // Create a new instance of the search bar UI - clearSearch(editor._codeMirror); + clearSearch(editor); doSearch(editor, false); } } diff --git a/src/search/ScrollTrackMarkers.js b/src/search/ScrollTrackMarkers.js index 2c5a72e644..79a7358569 100644 --- a/src/search/ScrollTrackMarkers.js +++ b/src/search/ScrollTrackMarkers.js @@ -23,57 +23,24 @@ * Manages tickmarks shown along the scrollbar track. * NOT yet intended for use by anyone other than the FindReplace module. * It is assumed that markers are always clear()ed when switching editors. + * + * Modified to allow each Editor to store its own scroll track markers via + * `editor._scrollTrackMarker`. */ define(function (require, exports, module) { - - var _ = require("thirdparty/lodash"); - - var WorkspaceManager = require("view/WorkspaceManager"); - - - /** - * Editor the markers are currently shown for, or null if not shown - * @type {?Editor} - * @private - */ - var editor; + const _ = require("thirdparty/lodash"), + EditorManager = require("editor/EditorManager"), + WorkspaceManager = require("view/WorkspaceManager"); /** - * Top of scrollbar track area, relative to top of scrollbar + * Vertical space above/below the scrollbar (set per OS). + * This remains global but applies to all editors. * @type {number} - * @private */ - var trackOffset; - - /** - * Height of scrollbar track area - * @type {number} - * @private - */ - var trackHt; - - /** - * Text positions of markers - * @type {!{line: number, ch: number}} Array - * @private - */ - var marks = []; - - /** - * Tickmark markCurrent() last called on, or null if never called / called with -1. - * @type {?jQueryObject} - * @private - */ - var $markedTickmark; - - /** - * Vertical space above and below the scrollbar - * @type {number} - * @private - */ - var scrollbarTrackOffset; + let scrollbarTrackOffset; + // Initialize scrollbarTrackOffset based on platform switch (brackets.platform) { case "win": // Custom scrollbar CSS has no gap around the track scrollbarTrackOffset = 0; @@ -81,236 +48,322 @@ define(function (require, exports, module) { case "mac": // Native scrollbar has padding around the track scrollbarTrackOffset = 4; break; - case "linux": // Custom scrollbar CSS has assymmetrical gap; this approximates it + case "linux": // Custom scrollbar CSS has asymmetrical gap; approximate it scrollbarTrackOffset = 2; break; } /** - * Vertical space above and below the scrollbar. - * @return {number} amount Value in pixels + * The (fixed) height of each individual tickmark. + * @const + */ + const MARKER_HEIGHT = 2; + + /** + * Returns the vertical space above and below the scrollbar, which depends + * on OS or may be altered by other CSS/extension changes. + * @return {number} */ function getScrollbarTrackOffset() { return scrollbarTrackOffset; } /** - * Sets how much vertical space there's above and below the scrollbar, which depends - * on the OS and may also be affected by extensions + * Sets how much vertical space there is above and below the scrollbar. * @param {number} offset Value in pixels */ function setScrollbarTrackOffset(offset) { scrollbarTrackOffset = offset; } + /** + * Helper: get or create the scrollTrackMarker state object for an editor. + * @param {!Editor} editor + * @return {Object} A state object stored in editorInstance._scrollTrackMarker + */ + function _getMarkerState(editor) { + if (!editor._scrollTrackMarker) { + editor._scrollTrackMarker = { + // Track geometry + trackOffset: 0, + trackHt: 0, + + // All marker positions + marks: [], - function _getScrollbar(editorInstance) { - // Be sure to select only the direct descendant, not also elements within nested inline editors - return $(editorInstance.getRootElement()).children(".CodeMirror-vscrollbar"); + // The "current" marked tick + $markedTickmark: null, + + // Whether the track is visible + visible: false, + + // Handler for resizing + resizeHandler: null + }; + } + return editor._scrollTrackMarker; } /** - * Measure scrollbar track - * @private + * Return the scrollbar element for the given editor. + * (Select only the direct descendant so we don't get nested inline editors). + * @param {!Editor} editor + * @return {jQueryObject} */ - function _calcScaling() { - var $sb = _getScrollbar(editor); + function _getScrollbar(editor) { + return $(editor.getRootElement()).children(".CodeMirror-vscrollbar"); + } - trackHt = $sb[0].offsetHeight; + /** + * Measure and store the scrollbar track geometry in editorInstance._scrollTrackMarker. + * @param {!Editor} editor + */ + function _calcScaling(editor) { + var markerState = _getMarkerState(editor); + var $sb = _getScrollbar(editor); - if (trackHt > 0) { - trackOffset = getScrollbarTrackOffset(); - trackHt -= trackOffset * 2; + var trackHeight = $sb[0].offsetHeight; + if (trackHeight > 0) { + markerState.trackOffset = getScrollbarTrackOffset(); + markerState.trackHt = trackHeight - markerState.trackOffset * 2; } else { // No scrollbar: use the height of the entire code content var codeContainer = $(editor.getRootElement()) .find("> .CodeMirror-scroll > .CodeMirror-sizer > div > .CodeMirror-lines > div")[0]; - trackHt = codeContainer.offsetHeight; - trackOffset = codeContainer.offsetTop; + markerState.trackHt = codeContainer.offsetHeight; + markerState.trackOffset = codeContainer.offsetTop; } } - function _getTop(editorInstance, pos) { - let cm = editorInstance._codeMirror, - editorHt = cm.getScrollerElement().scrollHeight, - cursorTop; - - let wrapping = cm.getOption("lineWrapping"), - singleLineH = wrapping && cm.defaultTextHeight() * 1.5; - let curLine = pos.line; - let curLineObj = cm.getLineHandle(curLine); - if (wrapping && curLineObj.height > singleLineH) { + /** + * Compute the "top" position in the scrollbar track for a given text pos. + * @param {!Editor} editor + * @param {{line: number, ch: number}} pos + * @return {number} Y offset in scrollbar track + */ + function _getTop(editor, pos) { + var cm = editor._codeMirror; + var markerState = _getMarkerState(editor); + var editorHt = cm.getScrollerElement().scrollHeight; + var wrapping = cm.getOption("lineWrapping"); + + var cursorTop; + var singleLineH = wrapping && cm.defaultTextHeight() * 1.5; + var lineObj = cm.getLineHandle(pos.line); + if (wrapping && lineObj && lineObj.height > singleLineH) { + // For wrapped lines, measure the exact y-position of the character cursorTop = cm.charCoords(pos, "local").top; } else { - cursorTop = cm.heightAtLine(curLineObj, "local"); + // For unwrapped lines or lines with default height + cursorTop = cm.heightAtLine(pos.line, "local"); } - // return top - return (Math.round(cursorTop / editorHt * trackHt) + trackOffset) - 1; // 1px Centering correction - } - const MARKER_HEIGHT = 2; + var ratio = editorHt ? (cursorTop / editorHt) : 0; + // offset in the scrollbar track + return Math.round(ratio * markerState.trackHt) + markerState.trackOffset - 1; + } /** - * Add merged tickmarks to the scrollbar track - * @private + * Renders the given list of positions as merged tickmarks in the scrollbar track. + * @param {!Editor} editor + * @param {!Array.<{line: number, ch: number}>} posArray */ - function _renderMarks(posArray) { - let cm = editor._codeMirror, - editorHt = cm.getScrollerElement().scrollHeight; + function _renderMarks(editor, posArray) { + const cm = editor._codeMirror; + const markerState = _getMarkerState(editor); + const $track = $(".tickmark-track", editor.getRootElement()); + const editorHt = cm.getScrollerElement().scrollHeight; + + const wrapping = cm.getOption("lineWrapping"), + singleLineH = wrapping && cm.defaultTextHeight() * 1.5; - let wrapping = cm.getOption("lineWrapping"), - singleLineH = wrapping && cm.defaultTextHeight() * 1.5, - curLine = null, - curLineObj = null; + // For performance, precompute top for each mark + const markPositions = []; + let curLine = null, curLineObj = null; function getY(pos) { if (curLine !== pos.line) { curLine = pos.line; curLineObj = cm.getLineHandle(curLine); } - if (wrapping && curLineObj.height > singleLineH) { + if (wrapping && curLineObj && curLineObj.height > singleLineH) { return cm.charCoords(pos, "local").top; } return cm.heightAtLine(curLineObj, "local"); } - var markPositions = []; - - // Convert line positions to scrollbar positions posArray.forEach(function (pos) { - var cursorTop = getY(pos), - top = Math.round(cursorTop / editorHt * trackHt) + trackOffset; - top--; // Centering correction - markPositions.push({ top: top, bottom: top + MARKER_HEIGHT }); // Default height is 2px + const y = getY(pos); + const ratio = editorHt ? (y / editorHt) : 0; + const top = Math.round(ratio * markerState.trackHt) + markerState.trackOffset - 1; + // default 2px height => from top..(top+2) + markPositions.push({ top: top, bottom: top + MARKER_HEIGHT }); }); - // Sort positions by top coordinate - markPositions.sort((a, b) => a.top - b.top); + // Sort them by top coordinate + markPositions.sort(function (a, b) { return a.top - b.top; }); - // Merge overlapping or adjacent tickmarks - var mergedMarks = []; - markPositions.forEach(({ top, bottom }) => { + // Merge nearby or overlapping segments + const mergedMarks = []; + markPositions.forEach(function (m) { if (mergedMarks.length > 0) { - let lastMark = mergedMarks[mergedMarks.length - 1]; - - if (top <= lastMark.bottom + 1) { - // Extend the existing mark - lastMark.bottom = Math.max(lastMark.bottom, bottom); - lastMark.height = lastMark.bottom - lastMark.top; // Adjust height + const last = mergedMarks[mergedMarks.length - 1]; + // If overlapping or adjacent, merge them + if (m.top <= last.bottom + 1) { + last.bottom = Math.max(last.bottom, m.bottom); + last.height = last.bottom - last.top; return; } } - // Create a new mark - mergedMarks.push({ top, bottom, height: bottom - top }); + m.height = m.bottom - m.top; + mergedMarks.push(m); }); - // Generate and insert tickmarks into the DOM - var html = mergedMarks.map(({ top, height }) => - `
` - ).join(""); + // Build HTML + const html = mergedMarks.map(function (m) { + return "
"; + }).join(""); - $(".tickmark-track", editor.getRootElement()).append($(html)); + // Append to track + $track.append($(html)); } - /** - * Clear any markers in the editor's tickmark track, but leave it visible. Safe to call when - * tickmark track is not visible also. + * Clear any markers in the editor's tickmark track, leaving it visible if it was shown. + * Safe to call when the tickmark track is not visible also. + * @param {!Editor} editor */ - function clear() { - if (editor) { - $(".tickmark-track", editor.getRootElement()).empty(); - marks = []; - $markedTickmark = null; + function clear(editor) { + if(!editor) { + console.error("Calling ScrollTrackMarkers.clear without editor instance is deprecated."); + editor = EditorManager.getActiveEditor(); + } + const markerState = editor && editor._scrollTrackMarker; + if (!markerState) { + return; + } + $(".tickmark-track", editor.getRootElement()).empty(); + markerState.marks = []; + if (markerState.$markedTickmark) { + markerState.$markedTickmark.remove(); + markerState.$markedTickmark = null; } } /** - * Add or remove the tickmark track from the editor's UI + * Shows or hides the tickmark track for the given editor. + * @param {!Editor} editor + * @param {boolean} visible */ - function setVisible(curEditor, visible) { - // short-circuit no-ops - if ((visible && curEditor === editor) || (!visible && !editor)) { + function setVisible(editor, visible) { + const markerState = _getMarkerState(editor); + + // No-op if the current visibility state is the same + if (markerState.visible === visible) { return; } - if (visible) { - console.assert(!editor); - editor = curEditor; - - // Don't support inline editors yet - search inside them is pretty screwy anyway (#2110) - if (editor.isTextSubset()) { - return; - } - - var $sb = _getScrollbar(editor), - $overlay = $("
"); - $sb.parent().append($overlay); + markerState.visible = visible; - _calcScaling(); - - // Update tickmarks during editor resize (whenever resizing has paused/stopped for > 1/3 sec) - WorkspaceManager.on("workspaceUpdateLayout.ScrollTrackMarkers", _.debounce(function () { - if (marks.length) { - _calcScaling(); + if (visible) { + // Create the container track if not present + const $scrollbar = _getScrollbar(editor); + const $overlay = $("
"); + $scrollbar.parent().append($overlay); + + // Calculate scaling + _calcScaling(editor); + + // Resize handler (debounced) + markerState.resizeHandler = _.debounce(function () { + if (markerState.marks.length) { + _calcScaling(editor); + // Re-render $(".tickmark-track", editor.getRootElement()).empty(); - _renderMarks(marks); + _renderMarks(editor, markerState.marks); } - }, 300)); + }, 300); + + // Attach to workspace resizing + WorkspaceManager.on("workspaceUpdateLayout.ScrollTrackMarkers", markerState.resizeHandler); } else { - console.assert(editor === curEditor); - $(".tickmark-track", curEditor.getRootElement()).remove(); - editor = null; - marks = []; - WorkspaceManager.off("workspaceUpdateLayout.ScrollTrackMarkers"); + // Remove the track markup + $(".tickmark-track", editor.getRootElement()).remove(); + + // Detach resizing + if (markerState.resizeHandler) { + WorkspaceManager.off("workspaceUpdateLayout.ScrollTrackMarkers", markerState.resizeHandler); + markerState.resizeHandler = null; + } + + // Clear marks data + markerState.marks = []; + markerState.$markedTickmark = null; } } /** - * Add tickmarks to the editor's tickmark track, if it's visible - * @param {!Editor} curEditor - * @param {!{line:Number, ch:Number}} posArray + * Adds tickmarks for the given positions into the editor's tickmark track, if visible. + * @param {!Editor} editor + * @param {!Array.<{line: number, ch: number}>} posArray */ - function addTickmarks(curEditor, posArray) { - console.assert(editor === curEditor); - - marks = marks.concat(posArray); - _renderMarks(posArray); + function addTickmarks(editor, posArray) { + const markerState = _getMarkerState(editor); + if (!markerState.visible) { + return; + } + // Concat new positions + markerState.marks = markerState.marks.concat(posArray); + _renderMarks(editor, posArray); } /** - * @param {number} index Either -1, or an index into the array passed to addTickmarks() + * Highlights the "current" tickmark at the given index (into the marks array provided to addTickmarks), + * or clears if `index === -1`. + * @param {number} index + * @param {!Editor} editor */ - function markCurrent(index) { - // Remove previous highlight first - if ($markedTickmark) { - $markedTickmark.remove(); - $markedTickmark = null; + function markCurrent(index, editor) { + if(!editor) { + throw new Error("Calling ScrollTrackMarkers.markCurrent without editor instance is deprecated."); } - if (index !== -1) { - const parkPos = marks[index]; - const top = _getTop(editor, parkPos); - $markedTickmark = $( - `
`); - $(".tickmark-track ").append($markedTickmark); + const markerState = _getMarkerState(editor); + // Remove previous highlight + if (markerState.$markedTickmark) { + markerState.$markedTickmark.remove(); + markerState.$markedTickmark = null; + } + if (index === -1 || !markerState.marks[index]) { + return; } - } - // Private helper for unit tests - function _getTickmarks() { - return marks; + const top = _getTop(editor, markerState.marks[index]); + const $tick = $( + `
`); + + $(".tickmark-track", editor.getRootElement()).append($tick); + markerState.$markedTickmark = $tick; } + /** + * Private helper for unit tests + * @param {!Editor} editorInstance + * @return {!Array.<{line: number, ch: number}>} + */ + function _getTickmarks(editorInstance) { + const markerState = editorInstance && editorInstance._scrollTrackMarker; + return markerState ? markerState.marks : []; + } // For unit tests - exports._getTickmarks = _getTickmarks; - - exports.clear = clear; - exports.setVisible = setVisible; - exports.addTickmarks = addTickmarks; - exports.markCurrent = markCurrent; - - exports.getScrollbarTrackOffset = getScrollbarTrackOffset; - exports.setScrollbarTrackOffset = setScrollbarTrackOffset; + exports._getTickmarks = _getTickmarks; + + // public API + exports.clear = clear; + exports.setVisible = setVisible; + exports.addTickmarks = addTickmarks; + exports.markCurrent = markCurrent; + exports.getScrollbarTrackOffset = getScrollbarTrackOffset; + exports.setScrollbarTrackOffset = setScrollbarTrackOffset; }); diff --git a/src/styles/brackets.less b/src/styles/brackets.less index d3a4e02d18..a26628ac91 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -2628,6 +2628,7 @@ textarea.exclusions-editor { &.tickmark-current { background-color: #ed9823; z-index: 1; // ensure this one appears above overlapping sibling highlights + opacity: 1; } } From 0ee6b4b809330d5a2a5f14a00f7305b8c15f39bf Mon Sep 17 00:00:00 2001 From: abose Date: Sun, 16 Feb 2025 21:21:23 +0530 Subject: [PATCH 3/8] chore: add support for named scroll markers --- src/search/ScrollTrackMarkers.js | 133 +++++++++++++++++++++---------- src/styles/brackets.less | 21 ++--- 2 files changed, 102 insertions(+), 52 deletions(-) diff --git a/src/search/ScrollTrackMarkers.js b/src/search/ScrollTrackMarkers.js index 79a7358569..bdfa36ba7e 100644 --- a/src/search/ScrollTrackMarkers.js +++ b/src/search/ScrollTrackMarkers.js @@ -33,6 +33,11 @@ define(function (require, exports, module) { EditorManager = require("editor/EditorManager"), WorkspaceManager = require("view/WorkspaceManager"); + const TRACK_STYLES = { + LINE: "line", + ON_LEFT: "left" + }; + /** * Vertical space above/below the scrollbar (set per OS). * This remains global but applies to all editors. @@ -57,24 +62,8 @@ define(function (require, exports, module) { * The (fixed) height of each individual tickmark. * @const */ - const MARKER_HEIGHT = 2; - - /** - * Returns the vertical space above and below the scrollbar, which depends - * on OS or may be altered by other CSS/extension changes. - * @return {number} - */ - function getScrollbarTrackOffset() { - return scrollbarTrackOffset; - } - - /** - * Sets how much vertical space there is above and below the scrollbar. - * @param {number} offset Value in pixels - */ - function setScrollbarTrackOffset(offset) { - scrollbarTrackOffset = offset; - } + const MARKER_HEIGHT_LINE = 2; + const MARKER_HEIGHT_LEFT = 5; /** * Helper: get or create the scrollTrackMarker state object for an editor. @@ -124,7 +113,7 @@ define(function (require, exports, module) { var trackHeight = $sb[0].offsetHeight; if (trackHeight > 0) { - markerState.trackOffset = getScrollbarTrackOffset(); + markerState.trackOffset = scrollbarTrackOffset; markerState.trackHt = trackHeight - markerState.trackOffset * 2; } else { // No scrollbar: use the height of the entire code content @@ -166,7 +155,7 @@ define(function (require, exports, module) { /** * Renders the given list of positions as merged tickmarks in the scrollbar track. * @param {!Editor} editor - * @param {!Array.<{line: number, ch: number}>} posArray + * @param {Array.<{line: number, ch: number}>} posArray */ function _renderMarks(editor, posArray) { const cm = editor._codeMirror; @@ -197,31 +186,47 @@ define(function (require, exports, module) { const ratio = editorHt ? (y / editorHt) : 0; const top = Math.round(ratio * markerState.trackHt) + markerState.trackOffset - 1; // default 2px height => from top..(top+2) - markPositions.push({ top: top, bottom: top + MARKER_HEIGHT }); + let markerHeight = MARKER_HEIGHT_LINE, isLine = true; + if(pos.options.trackStyle === TRACK_STYLES.ON_LEFT) { + markerHeight = MARKER_HEIGHT_LEFT; + isLine = false; + } + markPositions.push({ top: top, bottom: top + markerHeight, isLine, + cssColorClass: pos.options.cssColorClass || ""}); }); // Sort them by top coordinate markPositions.sort(function (a, b) { return a.top - b.top; }); // Merge nearby or overlapping segments - const mergedMarks = []; - markPositions.forEach(function (m) { + const mergedLineMarks = [], mergedLeftMarks = []; + markPositions.forEach(function (mark) { + const mergedMarks = mark.isLine ? mergedLineMarks : mergedLeftMarks; if (mergedMarks.length > 0) { const last = mergedMarks[mergedMarks.length - 1]; // If overlapping or adjacent, merge them - if (m.top <= last.bottom + 1) { - last.bottom = Math.max(last.bottom, m.bottom); + if (mark.top <= last.bottom + 1) { + last.bottom = Math.max(last.bottom, mark.bottom); last.height = last.bottom - last.top; return; } } - m.height = m.bottom - m.top; - mergedMarks.push(m); + mark.height = mark.bottom - mark.top; + mergedMarks.push(mark); }); - // Build HTML - const html = mergedMarks.map(function (m) { - return "
"; + // Build HTML for horizontal marks + let html = mergedLineMarks.map(function (m) { + return `
`; + }).join(""); + + // Append to track + $track.append($(html)); + + // Build HTML for hertical marks + html = mergedLeftMarks.map(function (m) { + return `
`; }).join(""); // Append to track @@ -229,27 +234,65 @@ define(function (require, exports, module) { } /** - * Clear any markers in the editor's tickmark track, leaving it visible if it was shown. - * Safe to call when the tickmark track is not visible also. + * Clear tickmarks from the editor's tickmark track. + * - If `markName` is provided, only clears marks with that name. + * - If `markName` is omitted, clears **all unnamed marks** but leaves named marks. + * - To **clear all marks**, including named ones, use `clearAll()`. * @param {!Editor} editor + * @param {string} [markName] Optional. If given, only clears marks with that name. */ - function clear(editor) { - if(!editor) { - console.error("Calling ScrollTrackMarkers.clear without editor instance is deprecated."); + function clear(editor, markName) { + if (!editor) { + console.error("Calling ScrollTrackMarkers.clear without an editor instance is deprecated."); editor = EditorManager.getActiveEditor(); } const markerState = editor && editor._scrollTrackMarker; if (!markerState) { return; } + + if (markName) { + // Filter out only the named marks that match the given `markName` + markerState.marks = markerState.marks.filter(mark => mark.options && mark.options.name !== markName); + } else { + // Remove only unnamed marks (marks where options.name is undefined or null) + markerState.marks = markerState.marks.filter(mark => mark.options && mark.options.name); + } + + // Re-render the marks after clearing $(".tickmark-track", editor.getRootElement()).empty(); + _renderMarks(editor, markerState.marks); + + if (markerState.$markedTickmark && markName) { + markerState.$markedTickmark.remove(); + markerState.$markedTickmark = null; + } + } + + /** + * Clears all tickmarks from the editor's tickmark track, including named and unnamed marks. + * @param {!Editor} editor + */ + function clearAll(editor) { + if (!editor) { + throw new Error("Called ScrollTrackMarkers.clearAll without an editor!"); + } + const markerState = editor && editor._scrollTrackMarker; + if (!markerState) { + return; + } + + // Completely remove all tickmarks markerState.marks = []; + $(".tickmark-track", editor.getRootElement()).empty(); + if (markerState.$markedTickmark) { markerState.$markedTickmark.remove(); markerState.$markedTickmark = null; } } + /** * Shows or hides the tickmark track for the given editor. * @param {!Editor} editor @@ -306,16 +349,22 @@ define(function (require, exports, module) { /** * Adds tickmarks for the given positions into the editor's tickmark track, if visible. * @param {!Editor} editor - * @param {!Array.<{line: number, ch: number}>} posArray + * @param {Array.<{line: number, ch: number}>} posArray + * @param {Object} [options] + * @param {string} [options.name] you can assign a name to marks and then use this name to selectively + * these clear marks. + * @param {string} [options.trackStyle] one of TRACK_STYLES.* + * @param {string} [options.cssColorClass] a css class that should only override the --mark-color css var. */ - function addTickmarks(editor, posArray) { + function addTickmarks(editor, posArray, options = {}) { const markerState = _getMarkerState(editor); if (!markerState.visible) { return; } + const newPosArray = posArray.map(pos => ({ ...pos, options })); // Concat new positions - markerState.marks = markerState.marks.concat(posArray); - _renderMarks(editor, posArray); + markerState.marks = markerState.marks.concat(newPosArray); + _renderMarks(editor, markerState.marks); } /** @@ -340,7 +389,7 @@ define(function (require, exports, module) { const top = _getTop(editor, markerState.marks[index]); const $tick = $( - `
`); + `
`); $(".tickmark-track", editor.getRootElement()).append($tick); markerState.$markedTickmark = $tick; @@ -361,9 +410,9 @@ define(function (require, exports, module) { // public API exports.clear = clear; + exports.clearAll = clearAll; exports.setVisible = setVisible; exports.addTickmarks = addTickmarks; exports.markCurrent = markCurrent; - exports.getScrollbarTrackOffset = getScrollbarTrackOffset; - exports.setScrollbarTrackOffset = setScrollbarTrackOffset; + exports.TRACK_STYLES = TRACK_STYLES; }); diff --git a/src/styles/brackets.less b/src/styles/brackets.less index a26628ac91..e9d547bf1d 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -2617,12 +2617,13 @@ textarea.exclusions-editor { width: 12px; z-index: @z-index-cm-max; pointer-events: none; + --mark-color: #eddd23; .tickmark { position: absolute; width: 12px; height: 2px; - background-color: #eddd23; + background-color: var(--mark-color); opacity: 0.85; // allow thumb to show through &.tickmark-current { @@ -2630,19 +2631,19 @@ textarea.exclusions-editor { z-index: 1; // ensure this one appears above overlapping sibling highlights opacity: 1; } - } - .tickmark-side { - position: absolute; - width: 12px; - height: 5px; - border-left: 2px solid; - border-left-color: #e0d123; - opacity: 0.85; // allow thumb to show through + &.tickmark-side { + background-color: unset; + position: absolute; + width: 12px; + height: 5px; + border-left: 2px solid; + border-left-color: var(--mark-color); + opacity: 1; + } } } - /* Quick Open search bar & dropdown */ .find-dialog-label { From b612cf9e55c5a273df4018d83c41c3f57348cc74 Mon Sep 17 00:00:00 2001 From: abose Date: Sun, 16 Feb 2025 21:49:00 +0530 Subject: [PATCH 4/8] chore: deprecated ScrollTrackMarkers.setVisible() as Track visibility is now managed automatically --- src/search/ScrollTrackMarkers.js | 302 ++++++++++++++++++------------- 1 file changed, 181 insertions(+), 121 deletions(-) diff --git a/src/search/ScrollTrackMarkers.js b/src/search/ScrollTrackMarkers.js index bdfa36ba7e..a4cb26071e 100644 --- a/src/search/ScrollTrackMarkers.js +++ b/src/search/ScrollTrackMarkers.js @@ -2,7 +2,8 @@ * GNU AGPL-3.0 License * * Copyright (c) 2021 - present core.ai . All rights reserved. - * Original work Copyright (c) 2013 - 2021 Adobe Systems Incorporated. All rights reserved. + * Original work Copyright (c) 2013 - 2021 Adobe Systems Incorporated. + * All rights reserved. * * This program is free software: you can redistribute it and/or modify it * under the terms of the GNU Affero General Public License as published by @@ -21,16 +22,19 @@ /** * Manages tickmarks shown along the scrollbar track. - * NOT yet intended for use by anyone other than the FindReplace module. - * It is assumed that markers are always clear()ed when switching editors. + * NOT yet intended for use by anyone other than the internal modules. * * Modified to allow each Editor to store its own scroll track markers via * `editor._scrollTrackMarker`. + * + * Also modified so that the track visibility is managed automatically + * based on whether there are tickmarks present. The `setVisible()` method + * is now deprecated and should no longer be called directly. */ define(function (require, exports, module) { const _ = require("thirdparty/lodash"), - EditorManager = require("editor/EditorManager"), + EditorManager = require("editor/EditorManager"), WorkspaceManager = require("view/WorkspaceManager"); const TRACK_STYLES = { @@ -59,16 +63,22 @@ define(function (require, exports, module) { } /** - * The (fixed) height of each individual tickmark. + * The (fixed) height of each individual tickmark for "line" style. * @const */ const MARKER_HEIGHT_LINE = 2; + + /** + * The (fixed) height of each individual tickmark for "left" style. + * @const + */ const MARKER_HEIGHT_LEFT = 5; + /** * Helper: get or create the scrollTrackMarker state object for an editor. * @param {!Editor} editor - * @return {Object} A state object stored in editorInstance._scrollTrackMarker + * @return {Object} A state object stored in editor._scrollTrackMarker */ function _getMarkerState(editor) { if (!editor._scrollTrackMarker) { @@ -81,9 +91,9 @@ define(function (require, exports, module) { marks: [], // The "current" marked tick - $markedTickmark: null, + $currentTick: null, - // Whether the track is visible + // Whether the track is currently visible visible: false, // Handler for resizing @@ -104,20 +114,20 @@ define(function (require, exports, module) { } /** - * Measure and store the scrollbar track geometry in editorInstance._scrollTrackMarker. + * Measure and store the scrollbar track geometry in editor._scrollTrackMarker. * @param {!Editor} editor */ function _calcScaling(editor) { - var markerState = _getMarkerState(editor); - var $sb = _getScrollbar(editor); + const markerState = _getMarkerState(editor); + const $sb = _getScrollbar(editor); - var trackHeight = $sb[0].offsetHeight; + const trackHeight = $sb[0].offsetHeight; if (trackHeight > 0) { markerState.trackOffset = scrollbarTrackOffset; markerState.trackHt = trackHeight - markerState.trackOffset * 2; } else { // No scrollbar: use the height of the entire code content - var codeContainer = $(editor.getRootElement()) + const codeContainer = $(editor.getRootElement()) .find("> .CodeMirror-scroll > .CodeMirror-sizer > div > .CodeMirror-lines > div")[0]; markerState.trackHt = codeContainer.offsetHeight; markerState.trackOffset = codeContainer.offsetTop; @@ -131,14 +141,15 @@ define(function (require, exports, module) { * @return {number} Y offset in scrollbar track */ function _getTop(editor, pos) { - var cm = editor._codeMirror; - var markerState = _getMarkerState(editor); - var editorHt = cm.getScrollerElement().scrollHeight; - var wrapping = cm.getOption("lineWrapping"); - - var cursorTop; - var singleLineH = wrapping && cm.defaultTextHeight() * 1.5; - var lineObj = cm.getLineHandle(pos.line); + const cm = editor._codeMirror; + const markerState = _getMarkerState(editor); + const editorHt = cm.getScrollerElement().scrollHeight; + const wrapping = cm.getOption("lineWrapping"); + + let cursorTop; + const singleLineH = wrapping && cm.defaultTextHeight() * 1.5; + const lineObj = cm.getLineHandle(pos.line); + if (wrapping && lineObj && lineObj.height > singleLineH) { // For wrapped lines, measure the exact y-position of the character cursorTop = cm.charCoords(pos, "local").top; @@ -147,7 +158,7 @@ define(function (require, exports, module) { cursorTop = cm.heightAtLine(pos.line, "local"); } - var ratio = editorHt ? (cursorTop / editorHt) : 0; + const ratio = editorHt ? (cursorTop / editorHt) : 0; // offset in the scrollbar track return Math.round(ratio * markerState.trackHt) + markerState.trackOffset - 1; } @@ -163,8 +174,8 @@ define(function (require, exports, module) { const $track = $(".tickmark-track", editor.getRootElement()); const editorHt = cm.getScrollerElement().scrollHeight; - const wrapping = cm.getOption("lineWrapping"), - singleLineH = wrapping && cm.defaultTextHeight() * 1.5; + const wrapping = cm.getOption("lineWrapping"); + const singleLineH = wrapping && cm.defaultTextHeight() * 1.5; // For performance, precompute top for each mark const markPositions = []; @@ -185,21 +196,29 @@ define(function (require, exports, module) { const y = getY(pos); const ratio = editorHt ? (y / editorHt) : 0; const top = Math.round(ratio * markerState.trackHt) + markerState.trackOffset - 1; - // default 2px height => from top..(top+2) - let markerHeight = MARKER_HEIGHT_LINE, isLine = true; - if(pos.options.trackStyle === TRACK_STYLES.ON_LEFT) { + + let markerHeight = MARKER_HEIGHT_LINE; + let isLine = true; + if (pos.options.trackStyle === TRACK_STYLES.ON_LEFT) { markerHeight = MARKER_HEIGHT_LEFT; isLine = false; } - markPositions.push({ top: top, bottom: top + markerHeight, isLine, - cssColorClass: pos.options.cssColorClass || ""}); + + markPositions.push({ + top: top, + bottom: top + markerHeight, + isLine: isLine, + cssColorClass: pos.options.cssColorClass || "" + }); }); // Sort them by top coordinate markPositions.sort(function (a, b) { return a.top - b.top; }); // Merge nearby or overlapping segments - const mergedLineMarks = [], mergedLeftMarks = []; + const mergedLineMarks = []; + const mergedLeftMarks = []; + markPositions.forEach(function (mark) { const mergedMarks = mark.isLine ? mergedLineMarks : mergedLeftMarks; if (mergedMarks.length > 0) { @@ -219,22 +238,79 @@ define(function (require, exports, module) { let html = mergedLineMarks.map(function (m) { return `
`; }).join(""); - - // Append to track $track.append($(html)); - // Build HTML for hertical marks + // Build HTML for vertical marks html = mergedLeftMarks.map(function (m) { return `
`; }).join(""); - - // Append to track $track.append($(html)); } /** - * Clear tickmarks from the editor's tickmark track. + * Private helper: Show the track if it's not already visible. + * @param {!Editor} editor + */ + function _showTrack(editor) { + const markerState = _getMarkerState(editor); + if (markerState.visible) { + return; // Already visible + } + markerState.visible = true; + + // Create the container track if not present + const $scrollbar = _getScrollbar(editor); + const $overlay = $("
"); + $scrollbar.parent().append($overlay); + + // Calculate scaling + _calcScaling(editor); + + // Resize handler (debounced) + markerState.resizeHandler = _.debounce(function () { + if (markerState.marks.length) { + _calcScaling(editor); + // Re-render + $(".tickmark-track", editor.getRootElement()).empty(); + _renderMarks(editor, markerState.marks); + } + }, 300); + + // Attach to workspace resizing + WorkspaceManager.on("workspaceUpdateLayout.ScrollTrackMarkers", markerState.resizeHandler); + } + + /** + * Private helper: Hide the track if it's visible, and remove all markup. + * @param {!Editor} editor + */ + function _hideTrack(editor) { + const markerState = _getMarkerState(editor); + if (!markerState.visible) { + return; // Already hidden + } + markerState.visible = false; + + // Remove the track markup + $(".tickmark-track", editor.getRootElement()).remove(); + + // Detach resizing + if (markerState.resizeHandler) { + WorkspaceManager.off("workspaceUpdateLayout.ScrollTrackMarkers", markerState.resizeHandler); + markerState.resizeHandler = null; + } + + // Clear marks data (since track is gone, no need to keep them) + markerState.marks = []; + if (markerState.$currentTick) { + markerState.$currentTick.remove(); + markerState.$currentTick = null; + } + } + + /** + * Clears tickmarks from the editor's tickmark track. * - If `markName` is provided, only clears marks with that name. * - If `markName` is omitted, clears **all unnamed marks** but leaves named marks. * - To **clear all marks**, including named ones, use `clearAll()`. @@ -259,13 +335,17 @@ define(function (require, exports, module) { markerState.marks = markerState.marks.filter(mark => mark.options && mark.options.name); } - // Re-render the marks after clearing - $(".tickmark-track", editor.getRootElement()).empty(); - _renderMarks(editor, markerState.marks); + // After removing, either re-render or hide the track if no marks remain + if (markerState.marks.length === 0) { + _hideTrack(editor); + } else { + $(".tickmark-track", editor.getRootElement()).empty(); + _renderMarks(editor, markerState.marks); + } - if (markerState.$markedTickmark && markName) { - markerState.$markedTickmark.remove(); - markerState.$markedTickmark = null; + if (markerState.$currentTick && !markName) { + markerState.$currentTick.remove(); + markerState.$currentTick = null; } } @@ -284,87 +364,64 @@ define(function (require, exports, module) { // Completely remove all tickmarks markerState.marks = []; - $(".tickmark-track", editor.getRootElement()).empty(); - - if (markerState.$markedTickmark) { - markerState.$markedTickmark.remove(); - markerState.$markedTickmark = null; - } + _hideTrack(editor); } - /** - * Shows or hides the tickmark track for the given editor. - * @param {!Editor} editor - * @param {boolean} visible - */ - function setVisible(editor, visible) { - const markerState = _getMarkerState(editor); - - // No-op if the current visibility state is the same - if (markerState.visible === visible) { - return; - } - - markerState.visible = visible; - - if (visible) { - // Create the container track if not present - const $scrollbar = _getScrollbar(editor); - const $overlay = $("
"); - $scrollbar.parent().append($overlay); - - // Calculate scaling - _calcScaling(editor); - - // Resize handler (debounced) - markerState.resizeHandler = _.debounce(function () { - if (markerState.marks.length) { - _calcScaling(editor); - // Re-render - $(".tickmark-track", editor.getRootElement()).empty(); - _renderMarks(editor, markerState.marks); - } - }, 300); - - // Attach to workspace resizing - WorkspaceManager.on("workspaceUpdateLayout.ScrollTrackMarkers", markerState.resizeHandler); - - } else { - // Remove the track markup - $(".tickmark-track", editor.getRootElement()).remove(); + * DEPRECATED: Shows or hides the tickmark track for the given editor. + * + * The track is now automatically shown/hidden based on the presence of + * tickmarks. You generally no longer need to call this method. - // Detach resizing - if (markerState.resizeHandler) { - WorkspaceManager.off("workspaceUpdateLayout.ScrollTrackMarkers", markerState.resizeHandler); - markerState.resizeHandler = null; - } - - // Clear marks data - markerState.marks = []; - markerState.$markedTickmark = null; - } + * @deprecated + */ + function setVisible() { + console.warn("DEPRECATED: ScrollTrackMarkers.setVisible() is no longer needed. " + + "Track visibility is now managed automatically."); } /** - * Adds tickmarks for the given positions into the editor's tickmark track, if visible. + * Adds tickmarks for the given positions into the editor's tickmark track. + * If the track was not visible and new marks are added, it is automatically shown. * @param {!Editor} editor * @param {Array.<{line: number, ch: number}>} posArray * @param {Object} [options] * @param {string} [options.name] you can assign a name to marks and then use this name to selectively - * these clear marks. + * clear these marks. * @param {string} [options.trackStyle] one of TRACK_STYLES.* - * @param {string} [options.cssColorClass] a css class that should only override the --mark-color css var. + * @param {string} [options.cssColorClass] a css class that should override the --mark-color css var. */ function addTickmarks(editor, posArray, options = {}) { const markerState = _getMarkerState(editor); - if (!markerState.visible) { + if (!markerState) { return; } + + // Make sure we have a valid editor instance + if (!editor) { + console.error("Calling ScrollTrackMarkers.addTickmarks without an editor is deprecated."); + editor = EditorManager.getActiveEditor(); + } + + // If track was empty before, note it + const wasEmpty = (markerState.marks.length === 0); + + // Normalize the new positions to include the same options object const newPosArray = posArray.map(pos => ({ ...pos, options })); - // Concat new positions + + // Concat the new positions markerState.marks = markerState.marks.concat(newPosArray); - _renderMarks(editor, markerState.marks); + + // If we were empty and now have tickmarks, show track + if (wasEmpty && markerState.marks.length > 0) { + _showTrack(editor); + } + + // If track is visible, re-render + if (markerState.visible) { + $(".tickmark-track", editor.getRootElement()).empty(); + _renderMarks(editor, markerState.marks); + } } /** @@ -374,25 +431,28 @@ define(function (require, exports, module) { * @param {!Editor} editor */ function markCurrent(index, editor) { - if(!editor) { + if (!editor) { throw new Error("Calling ScrollTrackMarkers.markCurrent without editor instance is deprecated."); } const markerState = _getMarkerState(editor); + // Remove previous highlight - if (markerState.$markedTickmark) { - markerState.$markedTickmark.remove(); - markerState.$markedTickmark = null; + if (markerState.$currentTick) { + markerState.$currentTick.remove(); + markerState.$currentTick = null; } if (index === -1 || !markerState.marks[index]) { return; } + // Position the highlight const top = _getTop(editor, markerState.marks[index]); - const $tick = $( - `
`); + const $currentTick = $( + `
` + ); - $(".tickmark-track", editor.getRootElement()).append($tick); - markerState.$markedTickmark = $tick; + $(".tickmark-track", editor.getRootElement()).append($currentTick); + markerState.$currentTick = $currentTick; } /** @@ -406,13 +466,13 @@ define(function (require, exports, module) { } // For unit tests - exports._getTickmarks = _getTickmarks; - - // public API - exports.clear = clear; - exports.clearAll = clearAll; - exports.setVisible = setVisible; - exports.addTickmarks = addTickmarks; - exports.markCurrent = markCurrent; - exports.TRACK_STYLES = TRACK_STYLES; + exports._getTickmarks = _getTickmarks; + + // Public API + exports.clear = clear; + exports.clearAll = clearAll; + exports.setVisible = setVisible; // Deprecated + exports.addTickmarks = addTickmarks; + exports.markCurrent = markCurrent; + exports.TRACK_STYLES = TRACK_STYLES; }); From 17200a5d13678df0f904cfd15eccceab6e7df6b4 Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 17 Feb 2025 11:26:30 +0530 Subject: [PATCH 5/8] fix: integ tests failures due to scroll track marker refactor --- test/spec/FindReplace-integ-test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/spec/FindReplace-integ-test.js b/test/spec/FindReplace-integ-test.js index d853ce0004..a5799547ff 100644 --- a/test/spec/FindReplace-integ-test.js +++ b/test/spec/FindReplace-integ-test.js @@ -126,7 +126,9 @@ define(function (require, exports, module) { } // Verify number of tickmarks equals number of highlights - var tickmarks = tw$(".tickmark-track .tickmark", myEditor.getRootElement()); + var tickmarks = testWindow.brackets.test.ScrollTrackMarkers._getTickmarks(myEditor); + // we cannot compare the dom nodes like below as the algorithm will merge overlapping scrolltrackers. + // var tickmarks = tw$(".tickmark-track .tickmark:not(.tickmark-current)", myEditor.getRootElement()); expect(tickmarks.length).toEql(selections.length); // Verify that editor UI doesn't have extra ranges left highlighted from earlier @@ -395,7 +397,7 @@ define(function (require, exports, module) { enterSearchText("foo"); expectHighlightedMatches(fooExpectedMatches); - var marks = testWindow.brackets.test.ScrollTrackMarkers._getTickmarks(); + var marks = testWindow.brackets.test.ScrollTrackMarkers._getTickmarks(myEditor); expect(marks.length).toEql(fooExpectedMatches.length); marks.forEach(function (mark, index) { From 6dbfb4dd0497755d947e456e12694d77bc974d1b Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 17 Feb 2025 12:29:24 +0530 Subject: [PATCH 6/8] test: integ test for ScrollTrackMarker --- .../default/DebugCommands/MacroRunner.js | 15 +- test/UnitTestSuite.js | 1 + test/spec/ScrollTrackHandler-integ-test.js | 165 ++++++++++++++++++ 3 files changed, 176 insertions(+), 5 deletions(-) create mode 100644 test/spec/ScrollTrackHandler-integ-test.js diff --git a/src/extensions/default/DebugCommands/MacroRunner.js b/src/extensions/default/DebugCommands/MacroRunner.js index 60ef9361cf..f9ce68d710 100644 --- a/src/extensions/default/DebugCommands/MacroRunner.js +++ b/src/extensions/default/DebugCommands/MacroRunner.js @@ -498,10 +498,12 @@ define(function (require, exports, module) { /** * Opens a file in the first pane (left/top) * @param {string} filePath - Project relative or absolute file path + * @param {boolean} [addToWorkingSet] - true to add to working set * @returns {Promise} A promise that resolves when the file is opened */ - openFileInFirstPane: function(filePath) { - return jsPromise(CommandManager.execute(Commands.FILE_OPEN, { + openFileInFirstPane: function(filePath, addToWorkingSet) { + const command = addToWorkingSet ? Commands.CMD_ADD_TO_WORKINGSET_AND_OPEN : Commands.FILE_OPEN; + return jsPromise(CommandManager.execute(command, { fullPath: _getFullPath(filePath), paneId: "first-pane" })); @@ -510,10 +512,12 @@ define(function (require, exports, module) { /** * Opens a file in the second pane (right/bottom) * @param {string} filePath - Project relative or absolute file path + * @param {boolean} addToWorkingSet - true to add to working set * @returns {Promise} A promise that resolves when the file is opened */ - openFileInSecondPane: function(filePath) { - return jsPromise(CommandManager.execute(Commands.FILE_OPEN, { + openFileInSecondPane: function(filePath, addToWorkingSet) { + const command = addToWorkingSet ? Commands.CMD_ADD_TO_WORKINGSET_AND_OPEN : Commands.FILE_OPEN; + return jsPromise(CommandManager.execute(command, { fullPath: _getFullPath(filePath), paneId: "second-pane" })); @@ -772,7 +776,8 @@ define(function (require, exports, module) { closeFile, closeAll, undo, redo, setPreference, getPreference, validateEqual, validateNotEqual, execCommand, saveActiveFile, awaitsFor, waitForModalDialog, waitForModalDialogClosed, clickDialogButtonID, clickDialogButton, - EDITING, $, Commands, Dialogs + EDITING, // contains apis like splitVertical, openFileInFirstPane. focus pane etc... + $, Commands, Dialogs }; async function runMacro(macroText) { diff --git a/test/UnitTestSuite.js b/test/UnitTestSuite.js index cd6663fc53..4fd8b5221d 100644 --- a/test/UnitTestSuite.js +++ b/test/UnitTestSuite.js @@ -119,6 +119,7 @@ define(function (require, exports, module) { require("spec/Generic-integ-test"); require("spec/spacing-auto-detect-integ-test"); require("spec/LocalizationUtils-test"); + require("spec/ScrollTrackHandler-integ-test"); // Integrated extension tests require("spec/Extn-InAppNotifications-integ-test"); require("spec/Extn-RemoteFileAdapter-integ-test"); diff --git a/test/spec/ScrollTrackHandler-integ-test.js b/test/spec/ScrollTrackHandler-integ-test.js new file mode 100644 index 0000000000..157469b945 --- /dev/null +++ b/test/spec/ScrollTrackHandler-integ-test.js @@ -0,0 +1,165 @@ +/* + * GNU AGPL-3.0 License + * + * Copyright (c) 2021 - present core.ai . All rights reserved. + * Original work Copyright (c) 2013 - 2021 Adobe Systems Incorporated. All rights reserved. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License + * for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://opensource.org/licenses/AGPL-3.0. + * + */ + +/*global describe, beforeEach, it, expect, beforeAll, afterAll */ + +define(function (require, exports, module) { + + + // Load dependent modules + let ScrollTrackMarkers, + __PR, + SpecRunnerUtils = require("spec/SpecRunnerUtils"); + + + describe("integration:Scroll Track markers", function () { + + let testWindow, + $, editor1Large, editor2; + + let currentProjectPath; + + beforeAll(async function () { + testWindow = await SpecRunnerUtils.createTestWindowAndRun(); + $ = testWindow.$; + + // Load module instances from brackets.test + ScrollTrackMarkers = testWindow.brackets.test.ScrollTrackMarkers; + __PR = testWindow.__PR; + currentProjectPath = await SpecRunnerUtils.getTestPath("/spec/HTMLInstrumentation-test-files"); + await SpecRunnerUtils.loadProjectInTestWindow(currentProjectPath); + await __PR.EDITING.splitHorizontal(); + await __PR.EDITING.openFileInFirstPane("REC-widgets-20121127.html", true); + await __PR.EDITING.openFileInSecondPane("omitEndTags.html", true); + editor1Large = __PR.EDITING.getFirstPaneEditor(); + editor2 = __PR.EDITING.getSecondPaneEditor(); + }, 30000); + + afterAll(async function () { + await SpecRunnerUtils.parkProject(true); + await __PR.EDITING.splitNone(); + testWindow = null; + __PR = null; + await SpecRunnerUtils.closeTestWindow(); + }, 30000); + + beforeEach(function () { + ScrollTrackMarkers.clearAll(editor1Large); + ScrollTrackMarkers.clearAll(editor2); + }); + + + it("should be able to add a line and side mark to both editors and clearAll", async function () { + const trackerList = [{line: 10, ch: 0}]; + ScrollTrackMarkers.addTickmarks(editor1Large, trackerList); + expect($(".tickmark").length).toBe(1); + ScrollTrackMarkers.addTickmarks(editor1Large, trackerList, { + trackStyle: ScrollTrackMarkers.TRACK_STYLES.ON_LEFT + }); + expect($(".tickmark").length).toBe(2); + expect($(".tickmark-side").length).toBe(1); + ScrollTrackMarkers.addTickmarks(editor2, trackerList); + expect($(".tickmark").length).toBe(3); + expect($(".tickmark-side").length).toBe(1); + ScrollTrackMarkers.addTickmarks(editor2, trackerList, { + trackStyle: ScrollTrackMarkers.TRACK_STYLES.ON_LEFT + }); + expect($(".tickmark").length).toBe(4); + expect($(".tickmark-side").length).toBe(2); + + // now clear all + ScrollTrackMarkers.clearAll(editor1Large); + expect($(".tickmark").length).toBe(2); + expect($(".tickmark-side").length).toBe(1); + ScrollTrackMarkers.clearAll(editor2); + expect($(".tickmark").length).toBe(0); + expect($(".tickmark-side").length).toBe(0); + }); + + it("should be able to add and clear named marks", async function () { + const trackerList = [{line: 10, ch: 0}]; + const trackName1= "line_ed_1"; + const trackName2= "side_ed_1"; + ScrollTrackMarkers.addTickmarks(editor1Large, trackerList, { + name: trackName1 + }); + expect($(".tickmark").length).toBe(1); + ScrollTrackMarkers.addTickmarks(editor1Large, trackerList, { + trackStyle: ScrollTrackMarkers.TRACK_STYLES.ON_LEFT, + name: trackName2 + }); + expect($(".tickmark").length).toBe(2); + expect($(".tickmark-side").length).toBe(1); + + // now remove by name + ScrollTrackMarkers.clear(editor1Large, trackName1); + expect($(".tickmark").length).toBe(1); + expect($(".tickmark-side").length).toBe(1); + ScrollTrackMarkers.clear(editor1Large, trackName2); + expect($(".tickmark").length).toBe(0); + expect($(".tickmark-side").length).toBe(0); + }); + + it("should be able to add css classes marks", async function () { + const trackerList = [{line: 10, ch: 0}]; + const trackClass1= "line_ed_1"; + const trackClass2= "side_ed_1"; + ScrollTrackMarkers.addTickmarks(editor1Large, trackerList, { + name: trackClass1, + cssColorClass: trackClass1 + }); + expect($(`.tickmark.${trackClass1}`).length).toBe(1); + expect($(".tickmark").length).toBe(1); + ScrollTrackMarkers.addTickmarks(editor1Large, trackerList, { + trackStyle: ScrollTrackMarkers.TRACK_STYLES.ON_LEFT, + name: trackClass2, + cssColorClass: trackClass2 + }); + expect($(`.tickmark-side.${trackClass2}`).length).toBe(1); + expect($(".tickmark-side").length).toBe(1); + }); + + it("should merge nearby scroll marks to single mark", async function () { + const trackerList = [{line: 10, ch: 0}, {line: 11, ch: 0}]; + ScrollTrackMarkers.addTickmarks(editor1Large, trackerList); + expect($(".tickmark").length).toBe(1); + expect($(".tickmark-side").length).toBe(0); + ScrollTrackMarkers.addTickmarks(editor1Large, trackerList, { + trackStyle: ScrollTrackMarkers.TRACK_STYLES.ON_LEFT + }); + expect($(".tickmark").length).toBe(2); + expect($(".tickmark-side").length).toBe(1); + }); + + it("should not merge far away scroll marks to single mark", async function () { + const trackerList = [{line: 10, ch: 0}, {line: 2000, ch: 0}]; + ScrollTrackMarkers.addTickmarks(editor1Large, trackerList); + expect($(".tickmark").length).toBe(2); + expect($(".tickmark-side").length).toBe(0); + ScrollTrackMarkers.addTickmarks(editor1Large, trackerList, { + trackStyle: ScrollTrackMarkers.TRACK_STYLES.ON_LEFT + }); + expect($(".tickmark").length).toBe(4); + expect($(".tickmark-side").length).toBe(2); + }); + + }); +}); From 58e784a38400f7a8bf85be37a6525d4e5f3fa91c Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 17 Feb 2025 12:40:42 +0530 Subject: [PATCH 7/8] fix: intermittant integ tests fails --- test/spec/TaskManager-integ-test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/spec/TaskManager-integ-test.js b/test/spec/TaskManager-integ-test.js index 580d3c17cf..f828bdebdd 100644 --- a/test/spec/TaskManager-integ-test.js +++ b/test/spec/TaskManager-integ-test.js @@ -24,8 +24,7 @@ define(function (require, exports, module) { // Recommended to avoid reloading the integration test window Phoenix instance for each test. - const SpecRunnerUtils = require("spec/SpecRunnerUtils"), - Strings = require("strings"); + const SpecRunnerUtils = require("spec/SpecRunnerUtils"); const testPath = SpecRunnerUtils.getTestPath("/spec/JSUtils-test-files"); @@ -34,6 +33,7 @@ define(function (require, exports, module) { MainViewManager, TaskManager, StatusBar, + Strings, PreferencesManager, CommandManager, Commands, @@ -56,6 +56,7 @@ define(function (require, exports, module) { TaskManager = brackets.test.TaskManager; StatusBar = brackets.test.StatusBar; PreferencesManager = brackets.test.PreferencesManager; + Strings = testWindow.Strings; await SpecRunnerUtils.loadProjectInTestWindow(testPath); }, 30000); From 60f5736d2019bf439a3bf99b10e8dd5392ae116d Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 17 Feb 2025 13:15:33 +0530 Subject: [PATCH 8/8] fix: integ jshint test fails with smaller files load here as large html lint may be causing probem --- test/spec/ScrollTrackHandler-integ-test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/spec/ScrollTrackHandler-integ-test.js b/test/spec/ScrollTrackHandler-integ-test.js index 157469b945..fb60e4339b 100644 --- a/test/spec/ScrollTrackHandler-integ-test.js +++ b/test/spec/ScrollTrackHandler-integ-test.js @@ -44,11 +44,11 @@ define(function (require, exports, module) { // Load module instances from brackets.test ScrollTrackMarkers = testWindow.brackets.test.ScrollTrackMarkers; __PR = testWindow.__PR; - currentProjectPath = await SpecRunnerUtils.getTestPath("/spec/HTMLInstrumentation-test-files"); + currentProjectPath = await SpecRunnerUtils.getTestPath("/spec/CSSUtils-test-files"); await SpecRunnerUtils.loadProjectInTestWindow(currentProjectPath); await __PR.EDITING.splitHorizontal(); - await __PR.EDITING.openFileInFirstPane("REC-widgets-20121127.html", true); - await __PR.EDITING.openFileInSecondPane("omitEndTags.html", true); + await __PR.EDITING.openFileInFirstPane("bootstrap.css", true); + await __PR.EDITING.openFileInSecondPane("variables.less", true); editor1Large = __PR.EDITING.getFirstPaneEditor(); editor2 = __PR.EDITING.getSecondPaneEditor(); }, 30000);