Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

[Breakpoints] Fix timing issue with showing breakpoints #7365

Merged
merged 3 commits into from Jan 8, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 5 additions & 18 deletions src/components/Editor/Breakpoint.js
Expand Up @@ -52,12 +52,9 @@ class Breakpoint extends PureComponent<Props> {

const sourceId = selectedSource.id;
const line = toEditorLine(sourceId, breakpoint.location.line);
const doc = getDocument(sourceId);

editor.codeMirror.setGutterMarker(
line,
"breakpoints",
makeMarker(breakpoint.disabled)
);
doc.setGutterMarker(line, "breakpoints", makeMarker(breakpoint.disabled));

editor.codeMirror.addLineClass(line, "line", "new-breakpoint");
if (breakpoint.condition) {
Expand All @@ -81,30 +78,20 @@ class Breakpoint extends PureComponent<Props> {

componentWillUnmount() {
const { editor, breakpoint, selectedSource } = this.props;

if (!selectedSource) {
return;
}

if (breakpoint.loading) {
if (!selectedSource || breakpoint.loading) {
return;
}

const sourceId = selectedSource.id;
const doc = getDocument(sourceId);

if (!doc) {
return;
}

const line = toEditorLine(sourceId, breakpoint.location.line);

// NOTE: when we upgrade codemirror we can use `doc.setGutterMarker`
if (doc.setGutterMarker) {
doc.setGutterMarker(line, "breakpoints", null);
} else {
editor.codeMirror.setGutterMarker(line, "breakpoints", null);
}

doc.setGutterMarker(line, "breakpoints", null);
doc.removeLineClass(line, "line", "new-breakpoint");
doc.removeLineClass(line, "line", "has-condition");
}
Expand Down
10 changes: 1 addition & 9 deletions src/components/Editor/Breakpoints.js
Expand Up @@ -20,18 +20,10 @@ type Props = {
};

class Breakpoints extends Component<Props> {
shouldComponentUpdate(nextProps: Props) {
if (nextProps.selectedSource && !isLoaded(nextProps.selectedSource)) {
return false;
}

return true;
}

render() {
const { breakpoints, selectedSource, editor } = this.props;

if (!selectedSource || !breakpoints || selectedSource.isBlackBoxed) {
if (!breakpoints || selectedSource.isBlackBoxed) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/Editor/index.js
Expand Up @@ -576,7 +576,7 @@ class Editor extends PureComponent<Props, State> {
const { horizontal, selectedSource, conditionalPanelLocation } = this.props;
const { editor } = this.state;

if (!editor || !selectedSource) {
if (!selectedSource || !editor || !getDocument(selectedSource.id)) {
Copy link
Contributor

@darkwing darkwing Jan 7, 2019

Choose a reason for hiding this comment

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

Two tests fail, each test containing the following line:

await waitForElementWithSelector(dbg, ".CodeMirror-code > .highlight-line");

The addition of || !getDocument(selectedSource.id) is causing the failure; if it is removed, the tests pass.

return null;
}

Expand Down