diff --git a/src/components/QuickOpenModal.js b/src/components/QuickOpenModal.js index 094346ef52..398da9fae5 100644 --- a/src/components/QuickOpenModal.js +++ b/src/components/QuickOpenModal.js @@ -7,6 +7,7 @@ import React, { Component } from "react"; import { connect } from "../utils/connect"; import fuzzyAldrin from "fuzzaldrin-plus"; import { basename } from "../utils/path"; +import { throttle } from "lodash"; import actions from "../actions"; import { @@ -43,8 +44,8 @@ import "./QuickOpenModal.css"; type Props = { enabled: boolean, - sources: Array, selectedSource?: Source, + displayedSources: Source[], query: string, searchType: QuickOpenType, symbols: FormattedSymbolDeclarations, @@ -69,12 +70,16 @@ type GotoLocationType = { column?: number }; +const updateResultsThrottle = 100; const maxResults = 100; function filter(values, query) { + const preparedQuery = fuzzyAldrin.prepareQuery(query); + return fuzzyAldrin.filter(values, query, { key: "value", - maxResults: maxResults + maxResults: maxResults, + preparedQuery }); } @@ -127,7 +132,9 @@ export class QuickOpenModal extends Component { }; searchSources = (query: string) => { - const { sources } = this.props; + const { displayedSources, tabs } = this.props; + const tabUrls = new Set(tabs.map((tab: Tab) => tab.url)); + const sources = formatSources(displayedSources, tabUrls); const results = query == "" ? sources : filter(sources, this.dropGoto(query)); return this.setResults(results); @@ -158,16 +165,24 @@ export class QuickOpenModal extends Component { }; showTopSources = () => { - const { tabs, sources } = this.props; + const { displayedSources, tabs } = this.props; + const tabUrls = new Set(tabs.map((tab: Tab) => tab.url)); + if (tabs.length > 0) { - const tabUrls = tabs.map((tab: Tab) => tab.url); - this.setResults(sources.filter(source => tabUrls.includes(source.url))); + this.setResults( + formatSources( + displayedSources.filter( + source => !!source.url && tabUrls.has(source.url) + ), + tabUrls + ) + ); } else { - this.setResults(sources); + this.setResults(formatSources(displayedSources, tabUrls)); } }; - updateResults = (query: string) => { + updateResults = throttle((query: string) => { if (this.isGotoQuery()) { return; } @@ -185,7 +200,7 @@ export class QuickOpenModal extends Component { } return this.searchSources(query); - }; + }, updateResultsThrottle); setModifier = (item: QuickOpenResult) => { if (["@", "#", ":"].includes(item.id)) { @@ -422,16 +437,17 @@ export class QuickOpenModal extends Component { /* istanbul ignore next: ignoring testing of redux connection stuff */ function mapStateToProps(state) { const selectedSource = getSelectedSource(state); + const tabs = getTabs(state); return { enabled: getQuickOpenEnabled(state), - sources: formatSources(getDisplayedSourcesList(state), getTabs(state)), selectedSource, + displayedSources: getDisplayedSourcesList(state), symbols: formatSymbols(getSymbols(state, selectedSource)), symbolsLoading: isSymbolsLoading(state, selectedSource), query: getQuickOpenQuery(state), searchType: getQuickOpenType(state), - tabs: getTabs(state) + tabs }; } diff --git a/src/components/test/QuickOpenModal.spec.js b/src/components/test/QuickOpenModal.spec.js index 169a4914c8..d3d7c95ee4 100644 --- a/src/components/test/QuickOpenModal.spec.js +++ b/src/components/test/QuickOpenModal.spec.js @@ -7,9 +7,14 @@ import React from "react"; import { shallow, mount } from "enzyme"; +import lodash from "lodash"; import { QuickOpenModal } from "../QuickOpenModal"; jest.mock("fuzzaldrin-plus"); +jest.unmock("lodash"); + +// $FlowIgnore +lodash.throttle = jest.fn(fn => fn); import { filter } from "fuzzaldrin-plus"; @@ -18,7 +23,7 @@ function generateModal(propOverrides, renderType = "shallow") { enabled: false, query: "", searchType: "sources", - sources: [], + displayedSources: [], tabs: [], selectSpecificLocation: jest.fn(), setQuickOpenQuery: jest.fn(), @@ -111,12 +116,24 @@ describe("QuickOpenModal", () => { { enabled: true, query: "", - sources: [{ url: "mozilla.com" }], + displayedSources: [ + // $FlowIgnore + { url: "mozilla.com", relativeUrl: true } + ], tabs: [generateTab("mozilla.com")] }, "shallow" ); - expect(wrapper.state("results")).toEqual([{ url: "mozilla.com" }]); + expect(wrapper.state("results")).toEqual([ + { + id: undefined, + icon: "tab result-item-icon", + subtitle: "true", + title: "mozilla.com", + url: "mozilla.com", + value: "true" + } + ]); }); describe("shows loading", () => { @@ -786,6 +803,7 @@ describe("QuickOpenModal", () => { }, "mount" ); + expect(wrapper).toMatchSnapshot(); }); diff --git a/src/components/test/__snapshots__/QuickOpenModal.spec.js.snap b/src/components/test/__snapshots__/QuickOpenModal.spec.js.snap index aa7eebf27b..4b291470b2 100644 --- a/src/components/test/__snapshots__/QuickOpenModal.spec.js.snap +++ b/src/components/test/__snapshots__/QuickOpenModal.spec.js.snap @@ -4,6 +4,7 @@ exports[`QuickOpenModal Basic render with mount & searchType = functions 1`] = ` > +) { const title = getFilename(source); const relativeUrlWithQuery = `${source.relativeUrl}${getSourceQueryString( source @@ -68,7 +71,7 @@ export function formatSourcesForList(source: Source, tabs: TabList) { value: relativeUrlWithQuery, title, subtitle: relativeUrlWithQuery, - icon: tabs.some(tab => tab.url == source.url) + icon: tabUrls.has(source.url) ? "tab result-item-icon" : classnames(getSourceClassnames(source), "result-item-icon"), id: source.id, @@ -136,10 +139,9 @@ export function formatShortcutResults(): Array { export function formatSources( sources: Source[], - tabs: TabList + tabUrls: Set<$PropertyType> ): Array { return sources - .filter(source => !isPretty(source)) - .filter(({ relativeUrl }) => !!relativeUrl) - .map(source => formatSourcesForList(source, tabs)); + .filter(source => !!source.relativeUrl && !isPretty(source)) + .map(source => formatSourcesForList(source, tabUrls)); } diff --git a/src/utils/result-list.js b/src/utils/result-list.js index 5bddc82fad..d9247b017b 100644 --- a/src/utils/result-list.js +++ b/src/utils/result-list.js @@ -42,12 +42,18 @@ function chromeScrollList(elem: Element, index: number): void { return; } - const resultsHeight: number = resultsEl.clientHeight; - const itemHeight: number = resultsEl.children[0].clientHeight; - const numVisible: number = resultsHeight / itemHeight; - const positionsToScroll: number = index - numVisible + 1; - const itemOffset: number = resultsHeight % itemHeight; - const scroll: number = positionsToScroll * (itemHeight + 2) + itemOffset; - - resultsEl.scrollTop = Math.max(0, scroll); + // Avoid expensive DOM computations (reading clientHeight) + // https://nolanlawson.com/2018/09/25/accurately-measuring-layout-on-the-web/ + requestAnimationFrame(() => { + setTimeout(() => { + const resultsHeight: number = resultsEl.clientHeight; + const itemHeight: number = resultsEl.children[0].clientHeight; + const numVisible: number = resultsHeight / itemHeight; + const positionsToScroll: number = index - numVisible + 1; + const itemOffset: number = resultsHeight % itemHeight; + const scroll: number = positionsToScroll * (itemHeight + 2) + itemOffset; + + resultsEl.scrollTop = Math.max(0, scroll); + }); + }); } diff --git a/test/mochitest/browser_dbg-quick-open.js b/test/mochitest/browser_dbg-quick-open.js index 653fdf2627..2a2a068441 100644 --- a/test/mochitest/browser_dbg-quick-open.js +++ b/test/mochitest/browser_dbg-quick-open.js @@ -47,10 +47,13 @@ function resultCount(dbg) { return findAllElements(dbg, "resultItems").length; } -function quickOpen(dbg, query, shortcut = "quickOpen") { +async function quickOpen(dbg, query, shortcut = "quickOpen") { pressKey(dbg, shortcut); assertEnabled(dbg); + query !== "" && type(dbg, query); + + await waitForTime(150); } function findResultEl(dbg, index = 1) { @@ -67,44 +70,45 @@ add_task(async function() { const dbg = await initDebugger("doc-script-switching.html"); info("test opening and closing"); - quickOpen(dbg, ""); + await quickOpen(dbg, ""); pressKey(dbg, "Escape"); assertDisabled(dbg); info("Testing the number of results for source search"); - quickOpen(dbg, "sw"); + await quickOpen(dbg, "sw"); is(resultCount(dbg), 2, "two file results"); pressKey(dbg, "Escape"); info("Testing source search and check to see if source is selected"); await waitForSource(dbg, "switching-01"); - quickOpen(dbg, "sw1"); + await quickOpen(dbg, "sw1"); is(resultCount(dbg), 1, "one file results"); pressKey(dbg, "Enter"); await waitForSelectedSource(dbg, "switching-01"); info("Test that results show tab icons"); - quickOpen(dbg, "sw1"); + await quickOpen(dbg, "sw1"); await assertResultIsTab(dbg, 1); pressKey(dbg, "Tab"); info("Testing arrow keys in source search and check to see if source is selected"); - quickOpen(dbg, "sw2"); + await quickOpen(dbg, "sw2"); is(resultCount(dbg), 1, "one file results"); pressKey(dbg, "Down"); pressKey(dbg, "Enter"); await waitForSelectedSource(dbg, "switching-02"); info("Testing tab closes the search"); - quickOpen(dbg, "sw"); + await quickOpen(dbg, "sw"); pressKey(dbg, "Tab"); assertDisabled(dbg); info("Testing function search"); - quickOpen(dbg, "", "quickOpenFunc"); + await quickOpen(dbg, "", "quickOpenFunc"); is(resultCount(dbg), 2, "two function results"); type(dbg, "@x"); + await waitForTime(150); is(resultCount(dbg), 0, "no functions with 'x' in name"); pressKey(dbg, "Escape"); @@ -113,13 +117,13 @@ add_task(async function() { info("Testing goto line:column"); assertLine(dbg, 0); assertColumn(dbg, null); - quickOpen(dbg, ":7:12"); + await quickOpen(dbg, ":7:12"); pressKey(dbg, "Enter"); assertLine(dbg, 7); assertColumn(dbg, 12); info("Testing gotoSource"); - quickOpen(dbg, "sw1:5"); + await quickOpen(dbg, "sw1:5"); pressKey(dbg, "Enter"); await waitForSelectedSource(dbg, "switching-01"); assertLine(dbg, 5);