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

Fix tabs reselect #2799

Merged
merged 4 commits into from
May 3, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion flow-typed/debugger-html.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ declare module "debugger-html" {
declare type SourceText = {
id: string,
text: string,
contentType: string
contentType: string,
loading?: boolean
};

/**
Expand Down
61 changes: 38 additions & 23 deletions src/components/Editor/Tabs.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// @flow

import { DOM as dom, PropTypes, PureComponent, createFactory } from "react";
import ImPropTypes from "react-immutable-proptypes";
import { DOM as dom, PureComponent, createFactory } from "react";
import { connect } from "react-redux";
import { bindActionCreators } from "redux";
import * as I from "immutable";

import {
getSelectedSource,
getSourcesForTabs,
Expand All @@ -25,6 +26,10 @@ const PaneToggleButton = createFactory(_PaneToggleButton);
import _Dropdown from "../shared/Dropdown";
const Dropdown = createFactory(_Dropdown);

import type { List } from "immutable";
import type { SourceRecord } from "../../reducers/sources";
type SourcesList = List<SourceRecord>;

/*
* Finds the hidden tabs by comparing the tabs' top offset.
* hidden tabs will have a great top offset.
Expand All @@ -34,7 +39,7 @@ const Dropdown = createFactory(_Dropdown);
*
* @returns Immutable.list
*/
function getHiddenTabs(sourceTabs, sourceTabEls) {
function getHiddenTabs(sourceTabs: SourcesList, sourceTabEls) {
sourceTabEls = [].slice.call(sourceTabEls);
function getTopOffset() {
const topOffsets = sourceTabEls.map(t => t.getBoundingClientRect().top);
Expand Down Expand Up @@ -66,7 +71,7 @@ function copyToTheClipboard(string) {

type State = {
dropdownShown: boolean,
hiddenSourceTabs: Array<Object> | null
hiddenSourceTabs: SourcesList
};

class SourceTabs extends PureComponent {
Expand All @@ -82,13 +87,29 @@ class SourceTabs extends PureComponent {
renderDropDown: Function;
renderStartPanelToggleButton: Function;
renderEndPanelToggleButton: Function;

props: {
sourceTabs: SourcesList,
selectedSource: SourceRecord,
selectSource: (string, ?Object) => any,
closeTab: string => any,
closeTabs: List<string> => any,
toggleProjectSearch: () => any,
togglePrettyPrint: string => any,
togglePaneCollapse: () => any,
showSource: string => any,
horizontal: boolean,
startPanelCollapsed: boolean,
endPanelCollapsed: boolean
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

switches from proptypes to props


onResize: Function;

constructor(props) {
super(props);
this.state = {
dropdownShown: false,
hiddenSourceTabs: null
hiddenSourceTabs: I.List()
};

this.onTabContextMenu = this.onTabContextMenu.bind(this);
Expand Down Expand Up @@ -127,7 +148,7 @@ class SourceTabs extends PureComponent {
window.removeEventListener("resize", this.onResize);
}

onTabContextMenu(event, tab) {
onTabContextMenu(event, tab: string) {
event.preventDefault();
this.showContextMenu(event, tab);
}
Expand Down Expand Up @@ -166,6 +187,11 @@ class SourceTabs extends PureComponent {
const sourceTab = sourceTabs.find(t => t.get("id") == tab);
const tabURLs = sourceTabs.map(thisTab => thisTab.get("url"));
const otherTabURLs = otherTabs.map(thisTab => thisTab.get("url"));

if (!sourceTab) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

flow doesn't know that we'll have a source in the tab list...


const isPrettySource = isPretty(sourceTab.toJS());

const closeTabMenuItem = {
Expand Down Expand Up @@ -272,7 +298,7 @@ class SourceTabs extends PureComponent {
});
}

renderDropdownSource(source) {
renderDropdownSource(source: SourceRecord) {
const { selectSource } = this.props;
const filename = getFilename(source.toJS());

Expand All @@ -291,13 +317,17 @@ class SourceTabs extends PureComponent {

renderTabs() {
const sourceTabs = this.props.sourceTabs;
if (!sourceTabs) {
return;
}

return dom.div(
{ className: "source-tabs", ref: "sourceTabs" },
sourceTabs.map(this.renderTab)
);
}

renderTab(source) {
renderTab(source: SourceRecord) {
const { selectedSource, selectSource, closeTab } = this.props;
const filename = getFilename(source.toJS());
const active =
Expand Down Expand Up @@ -400,21 +430,6 @@ class SourceTabs extends PureComponent {
}
}

SourceTabs.propTypes = {
sourceTabs: ImPropTypes.list.isRequired,
selectedSource: ImPropTypes.map,
selectSource: PropTypes.func.isRequired,
closeTab: PropTypes.func.isRequired,
closeTabs: PropTypes.func.isRequired,
toggleProjectSearch: PropTypes.func.isRequired,
togglePrettyPrint: PropTypes.func.isRequired,
togglePaneCollapse: PropTypes.func.isRequired,
showSource: PropTypes.func.isRequired,
horizontal: PropTypes.bool.isRequired,
startPanelCollapsed: PropTypes.bool.isRequired,
endPanelCollapsed: PropTypes.bool.isRequired
};

SourceTabs.displayName = "SourceTabs";

module.exports = connect(
Expand Down
33 changes: 20 additions & 13 deletions src/reducers/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@ import { getPrettySourceURL } from "../utils/source";
import { prefs } from "../utils/prefs";

import type { Map, List } from "immutable";
import type { Source, Location } from "../types";
import type { Source, SourceText, Location } from "../types";
import type { Action } from "../actions/types";
import type { Record } from "../utils/makeRecord";

type SourceRecord = Record<Source>;
type Tab = string;
export type SourceRecord = Record<Source>;
export type SourceTextRecord = Record<SourceText>;
type SourcesMap = Map<string, SourceRecord>;
type SourceTextMap = Map<string, SourceTextRecord>;
type TabList = List<Tab>;

export type SourcesState = {
sources: SourcesMap,
Expand All @@ -35,8 +39,8 @@ export type SourcesState = {
column?: number
},
selectedLocation?: Location,
sourcesText: Map<string, any>,
tabs: List<any>
sourcesText: SourceTextMap,
tabs: TabList
};

export const State = makeRecord(
Expand Down Expand Up @@ -256,9 +260,8 @@ function getNewSelectedSourceId(state: SourcesState, availableTabs): string {
const leftNeighborIndex = Math.max(tabUrls.indexOf(selectedTabUrl) - 1, 0);
const lastAvailbleTabIndex = availableTabs.size - 1;
const newSelectedTabIndex = Math.min(leftNeighborIndex, lastAvailbleTabIndex);
let tabSource = state.sources.find(
source => source.get("url") === availableTabs.toJS()[newSelectedTabIndex]
);
const availableTab = availableTabs.toJS()[newSelectedTabIndex];
const tabSource = getSourceByUrlInSources(state.sources, availableTab);

if (tabSource) {
return tabSource.get("id");
Expand All @@ -284,11 +287,14 @@ export function getSource(state: OuterState, id: string) {
return getSourceInSources(getSources(state), id);
}

export function getSourceByURL(state: OuterState, url: string) {
export function getSourceByURL(state: OuterState, url: string): ?SourceRecord {
return getSourceByUrlInSources(state.sources.sources, url);
}

export function getSourceText(state: OuterState, id: ?string) {
export function getSourceText(
state: OuterState,
id: ?string
): ?SourceTextRecord {
if (id) {
return state.sources.sourcesText.get(id);
}
Expand Down Expand Up @@ -331,10 +337,11 @@ export const getSourceTabs = createSelector(
export const getSourcesForTabs = createSelector(
getSourceTabs,
getSources,
(tabs, sources) =>
tabs
.map(tab => getSourceByUrlInSources(sources, tabs))
.filter(source => source)
(tabs: TabList, sources: SourcesMap) => {
return tabs
.map(tab => getSourceByUrlInSources(sources, tab))
.filter(source => source);
}
);

export const getSelectedLocation = createSelector(
Expand Down
3 changes: 2 additions & 1 deletion src/test/integration/tests/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ const {
initDebugger,
reload,
selectSource,
findElement,
waitForDispatch
} = require("../utils");

function countTabs(dbg) {
return dbg.selectors.getSourceTabs(dbg.getState()).size;
return findElement(dbg, "sourceTabs").children.length;
}

module.exports = async function(ctx) {
Expand Down
3 changes: 2 additions & 1 deletion src/test/integration/utils/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ const selectors = {
sourceFooter: ".source-footer",
sourceNode: i => `.sources-list .tree-node:nth-child(${i}) .node`,
sourceNodes: ".sources-list .tree-node",
sourceArrow: i => `.sources-list .tree-node:nth-child(${i}) .arrow`
sourceArrow: i => `.sources-list .tree-node:nth-child(${i}) .arrow`,
sourceTabs: `.source-tabs`
};

function findElement(dbg, elementName, ...args) {
Expand Down