Skip to content

Commit

Permalink
Bug 1878698 - [devtools] Avoid highlighting the location on cursor ch…
Browse files Browse the repository at this point in the history
…anges. r=devtools-reviewers,nchevobbe

This allows select text without having the highlighting going in the way.
It also prevents flashing the line when clicking around in the editor.

But we still update the location in the reducer in order to update the footer and outline panel
accordingly to the new cursor location.

Differential Revision: https://phabricator.services.mozilla.com/D200726
  • Loading branch information
ochameau committed Feb 21, 2024
1 parent 0b08110 commit 3077ef1
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 8 deletions.
16 changes: 13 additions & 3 deletions devtools/client/debugger/src/actions/sources/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,13 @@ import {
// This is only used by jest tests (and within this module)
export const setSelectedLocation = (
location,
shouldSelectOriginalLocation
shouldSelectOriginalLocation,
shouldHighlightSelectedLocation
) => ({
type: "SET_SELECTED_LOCATION",
location,
shouldSelectOriginalLocation,
shouldHighlightSelectedLocation,
});

// This is only used by jest tests (and within this module)
Expand Down Expand Up @@ -192,8 +194,14 @@ async function mayBeSelectMappedSource(location, keepContext, thunkArgs) {
* If false, this will ignore the currently selected source
* and select the generated or original location, even if we
* were currently selecting the other source type.
* @param {boolean} options.highlight
* True by default. To be set to false in order to preveng highlighting the selected location in the editor.
* We will only show the location, but do not put a special background on the line.
*/
export function selectLocation(location, { keepContext = true } = {}) {
export function selectLocation(
location,
{ keepContext = true, highlight = true } = {}
) {
return async thunkArgs => {
const { dispatch, getState, client } = thunkArgs;

Expand Down Expand Up @@ -245,7 +253,9 @@ export function selectLocation(location, { keepContext = true } = {}) {
dispatch(addTab(source, sourceActor));
}

dispatch(setSelectedLocation(location, shouldSelectOriginalLocation));
dispatch(
setSelectedLocation(location, shouldSelectOriginalLocation, highlight)
);

await dispatch(loadSourceText(source, sourceActor));

Expand Down
15 changes: 10 additions & 5 deletions devtools/client/debugger/src/components/Editor/HighlightLine.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
getSelectedSourceTextContent,
getPauseCommand,
getCurrentThread,
getShouldHighlightSelectedLocation,
} from "../../selectors/index";

function isDebugLine(selectedFrame, selectedLocation) {
Expand Down Expand Up @@ -95,6 +96,7 @@ export class HighlightLine extends Component {
selectedLocation,
selectedFrame,
selectedSourceTextContent,
shouldHighlightSelectedLocation,
} = this.props;
if (pauseCommand) {
this.isStepping = true;
Expand All @@ -107,11 +109,13 @@ export class HighlightLine extends Component {
prevProps.selectedSourceTextContent
);
}
this.setHighlightLine(
selectedLocation,
selectedFrame,
selectedSourceTextContent
);
if (shouldHighlightSelectedLocation) {
this.setHighlightLine(
selectedLocation,
selectedFrame,
selectedSourceTextContent
);
}
endOperation();
}

Expand Down Expand Up @@ -182,6 +186,7 @@ export default connect(state => {
}
return {
pauseCommand: getPauseCommand(state, getCurrentThread(state)),
shouldHighlightSelectedLocation: getShouldHighlightSelectedLocation(state),
selectedFrame: getVisibleSelectedFrame(state),
selectedLocation,
selectedSourceTextContent: getSelectedSourceTextContent(state),
Expand Down
3 changes: 3 additions & 0 deletions devtools/client/debugger/src/components/Editor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,9 @@ class Editor extends PureComponent {
// Reset the context, so that we don't switch to original
// while moving the cursor within a bundle
keepContext: false,

// Avoid highlighting the selected line
highlight: false,
}
);
};
Expand Down
8 changes: 8 additions & 0 deletions devtools/client/debugger/src/reducers/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ export function initialSourcesState(state) {
*/
selectedOriginalLocation: UNDEFINED_LOCATION,

/**
* By default, the `selectedLocation` should be highlighted in the editor with a special background.
* On demand, this flag can be set to false in order to prevent this.
* The location will be shown, but not highlighted.
*/
shouldHighlightSelectedLocation: true,

/**
* By default, if we have a source-mapped source, we would automatically try
* to select and show the content of the original source. But, if we explicitly
Expand Down Expand Up @@ -149,6 +156,7 @@ function update(state = initialSourcesState(), action) {
selectedOriginalLocation: UNDEFINED_LOCATION,
pendingSelectedLocation,
shouldSelectOriginalLocation: action.shouldSelectOriginalLocation,
shouldHighlightSelectedLocation: action.shouldHighlightSelectedLocation,
};
}

Expand Down
4 changes: 4 additions & 0 deletions devtools/client/debugger/src/selectors/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ export function getShouldSelectOriginalLocation(state) {
return state.sources.shouldSelectOriginalLocation;
}

export function getShouldHighlightSelectedLocation(state) {
return state.sources.shouldHighlightSelectedLocation;
}

/**
* Gets the first source actor for the source and/or thread
* provided.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ add_task(async function () {
2,
"when passing an explicit line number, the position is displayed"
);
assertHighlightLocation(dbg, "simple1.js", 1);

// Note that CodeMirror is 0-based while the footer displays 1-based
getCM(dbg).setCursor({ line: 1, ch: 0 });
await waitForCursorPosition(dbg, 2);
Expand All @@ -37,6 +39,11 @@ add_task(async function () {
1,
"when moving the cursor, the position footer updates"
);
ok(
!findElement(dbg, "highlightLine"),
"Moving the cursor resets the highlighted line"
);

getCM(dbg).setCursor({ line: 2, ch: 0 });
await waitForCursorPosition(dbg, 3);
assertCursorPosition(
Expand All @@ -56,6 +63,7 @@ add_task(async function () {
16,
"when moving the cursor a second time, the position footer still updates"
);
assertHighlightLocation(dbg, "simple1.js", 4);

info("Call the function that we set a breakpoint in.");
invokeInTab("main");
Expand Down

0 comments on commit 3077ef1

Please sign in to comment.