From 5dd237c1325b4c6981c3cafed44cf75d5118db03 Mon Sep 17 00:00:00 2001 From: Jason Laster Date: Fri, 9 Jun 2017 10:16:36 -0400 Subject: [PATCH] Fix 3146 - editor occassionaly doesn't jump to a line --- src/actions/sources.js | 9 ++++++++- src/actions/types.js | 3 ++- src/components/Editor/Tabs.js | 9 +++++---- src/components/Editor/index.js | 21 +++++++++++---------- src/components/stories/tabs.js | 1 + src/reducers/sources.js | 16 ++++++++-------- src/utils/editor/index.js | 5 ----- 7 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/actions/sources.js b/src/actions/sources.js index fa8654d9b0..aa5b60a8b6 100644 --- a/src/actions/sources.js +++ b/src/actions/sources.js @@ -178,7 +178,6 @@ export function selectSource(id: string, options: SelectSourceOptions = {}) { } let source = getSource(getState(), id); - if (!source) { // If there is no source we deselect the current selected source return dispatch({ type: "CLEAR_SELECTED_SOURCE" }); @@ -229,6 +228,14 @@ export function jumpToMappedLocation(sourceLocation: any) { }; } +export function moveTab(url: string, tabIndex: number) { + return { + type: "MOVE_TAB", + url, + tabIndex + }; +} + /** * @memberof actions/sources * @static diff --git a/src/actions/types.js b/src/actions/types.js index 6ff31fb966..2800019499 100644 --- a/src/actions/types.js +++ b/src/actions/types.js @@ -126,8 +126,9 @@ type SourceAction = frames: Frame[] } } + | { type: "MOVE_TAB", url: string, tabIndex: number } | { type: "CLOSE_TAB", url: string, tabs: any } - | { type: "CLOSE_TABS", urls: Array, tabs: any }; + | { type: "CLOSE_TABS", urls: string[], tabs: any }; export type panelPositionType = "start" | "end"; diff --git a/src/components/Editor/Tabs.js b/src/components/Editor/Tabs.js index ccaaeca908..75e85c4515 100644 --- a/src/components/Editor/Tabs.js +++ b/src/components/Editor/Tabs.js @@ -97,6 +97,7 @@ class SourceTabs extends PureComponent { sourceTabs: SourcesList, selectedSource: SourceRecord, selectSource: (string, ?Object) => any, + moveTab: (string, number) => any, closeTab: string => any, closeTabs: (List) => any, toggleProjectSearch: () => any, @@ -286,12 +287,12 @@ class SourceTabs extends PureComponent { if (!this.refs.sourceTabs) { return; } - const { selectedSource, sourceTabs, selectSource } = this.props; + const { selectedSource, sourceTabs, moveTab } = this.props; const sourceTabEls = this.refs.sourceTabs.children; const hiddenSourceTabs = getHiddenTabs(sourceTabs, sourceTabEls); if (hiddenSourceTabs.indexOf(selectedSource) !== -1) { - return selectSource(selectedSource.get("id"), { tabIndex: 0 }); + return moveTab(selectedSource.get("url"), 0); } this.setState({ hiddenSourceTabs }); @@ -304,7 +305,7 @@ class SourceTabs extends PureComponent { } renderDropdownSource(source: SourceRecord) { - const { selectSource } = this.props; + const { moveTab } = this.props; const filename = getFilename(source.toJS()); return dom.li( @@ -313,7 +314,7 @@ class SourceTabs extends PureComponent { onClick: () => { // const tabIndex = getLastVisibleTabIndex(sourceTabs, sourceTabEls); const tabIndex = 0; - selectSource(source.get("id"), { tabIndex }); + moveTab(source.get("url"), tabIndex); } }, filename diff --git a/src/components/Editor/index.js b/src/components/Editor/index.js index 5fe94ae7c5..b5e922e60a 100644 --- a/src/components/Editor/index.js +++ b/src/components/Editor/index.js @@ -65,7 +65,6 @@ import { shouldShowFooter, clearLineClass, createEditor, - isTextForSource, breakpointAtLocation, getTextForLine, getCursorLine, @@ -134,7 +133,7 @@ class Editor extends PureComponent { componentWillReceiveProps(nextProps) { // This lifecycle method is responsible for updating the editor // text. - const { sourceText, selectedLocation } = nextProps; + const { selectedSource, selectedLocation } = nextProps; this.clearDebugLine(this.props.selectedFrame); if ( @@ -144,14 +143,16 @@ class Editor extends PureComponent { this.editor.codeMirror.setSize(); } - if (!sourceText) { - if (this.props.sourceText) { + if (!selectedSource) { + if (this.props.selectedSource) { this.showMessage(""); } - } else if (!isTextForSource(sourceText)) { - this.showMessage(sourceText.get("error") || L10N.getStr("loadingText")); - } else if (this.props.sourceText !== sourceText) { - this.showSourceText(sourceText, selectedLocation); + } else if (selectedSource.get("loading")) { + this.showMessage(L10N.getStr("loadingText")); + } else if (selectedSource.get("error")) { + this.showMessage(selectedSource.get("error")); + } else if (this.props.selectedSource !== selectedSource) { + this.showSourceText(selectedSource, selectedLocation); } if (this.props.outOfScopeLocations !== nextProps.outOfScopeLocations) { @@ -258,7 +259,7 @@ class Editor extends PureComponent { // This is in `componentDidUpdate` so helper functions can expect // `this.props` to be the current props. This lifecycle method is // responsible for updating the editor annotations. - const { selectedLocation } = this.props; + const { selectedLocation, selectedSource } = this.props; // If the location is different and a new line is requested, // update the pending jump line. Note that if jumping to a line in @@ -275,7 +276,7 @@ class Editor extends PureComponent { // Only update and jump around in real source texts. This will // keep the jump state around until the real source text is // loaded. - if (this.props.sourceText && isTextForSource(this.props.sourceText)) { + if (selectedSource && selectedSource.has("text")) { this.highlightLine(); } } diff --git a/src/components/stories/tabs.js b/src/components/stories/tabs.js index f36b0be236..af11146d68 100644 --- a/src/components/stories/tabs.js +++ b/src/components/stories/tabs.js @@ -53,6 +53,7 @@ function TabsFactory(options, { dir = "ltr", theme = "light" } = {}) { searchOn: false, selectedSource: null, selectSource: action("selectSource"), + moveTab: action("moveTab"), closeTab: action("closeTab"), closeTabs: action("closeTabs"), toggleProjectSearch: action("toggleProjectSearch"), diff --git a/src/reducers/sources.js b/src/reducers/sources.js index 85d961d65d..f8630bf3fa 100644 --- a/src/reducers/sources.js +++ b/src/reducers/sources.js @@ -84,7 +84,6 @@ function update( }; prefs.pendingSelectedLocation = location; - return state .set("selectedLocation", { sourceId: action.source.id, @@ -92,11 +91,7 @@ function update( }) .set("pendingSelectedLocation", location) .merge({ - tabs: updateTabList( - { sources: state }, - action.source.url, - action.tabIndex - ) + tabs: updateTabList({ sources: state }, action.source.url) }); case "CLEAR_SELECTED_SOURCE": @@ -116,6 +111,11 @@ function update( prefs.pendingSelectedLocation = location; return state.set("pendingSelectedLocation", location); + case "MOVE_TAB": + return state.merge({ + tabs: updateTabList({ sources: state }, action.url, action.tabIndex) + }); + case "CLOSE_TAB": prefs.tabs = action.tabs; return state.merge({ tabs: action.tabs }); @@ -154,9 +154,9 @@ function getTextPropsFromAction(action: any) { const sourceText = action.value; if (action.status === "start") { - return { loading: true }; + return { id: source.id, loading: true }; } else if (action.status === "error") { - return { error: action.error, loading: false }; + return { id: source.id, error: action.error, loading: false }; } return { text: sourceText.text, diff --git a/src/utils/editor/index.js b/src/utils/editor/index.js index 4a6a7b84a7..6a4c105651 100644 --- a/src/utils/editor/index.js +++ b/src/utils/editor/index.js @@ -37,10 +37,6 @@ function shouldShowFooter(selectedSource, horizontal) { return shouldShowPrettyPrint(selectedSource); } -function isTextForSource(sourceText) { - return !sourceText.get("loading") && !sourceText.get("error"); -} - function breakpointAtLocation(breakpoints, { line, column = undefined }) { return breakpoints.find(bp => { const sameLine = bp.location.line === line + 1; @@ -118,7 +114,6 @@ module.exports = Object.assign( createEditor, shouldShowPrettyPrint, shouldShowFooter, - isTextForSource, breakpointAtLocation, traverseResults, updateDocument