WIP DO NOT REVIEW: Simpler options menu

config/webpack/base.config.ts

@@ -22,7 +22,7 @@ export default {
         phabricator: buildEntry(pageEntry, '../../src/libs/phabricator/extension.tsx'),
 
         bootstrap: path.join(__dirname, '../../node_modules/bootstrap/dist/css/bootstrap.css'),
-        style: path.join(__dirname, '../../src/shared/app.scss'),
+        style: path.join(__dirname, '../../src/app.scss'),
     },
     output: {
         path: path.join(__dirname, '../../build/dist/js'),

package.json

@@ -16,10 +16,10 @@
     "cypress:open": "yarn run build && cypress open",
     "test:e2e": "yarn run build && cypress run --browser chrome",
     "prettier": "prettier '**/{*.{js?(on),ts?(x),graphql,md,scss},.*.js?(on)}' --write --list-different --config prettier.config.js",
-    "storybook": "start-storybook -c ./config/storybook -p 6006",
-    "build-storybook": "build-storybook -c ./config/storybook",
-    "test": "mocha --require ts-node/register --watch --watch-extensions ts './src/**/*.test.ts?(x)'",
-    "test:ci": "mocha --require ts-node/register './src/**/*.test.ts?(x)'"
+    "test": "mocha --require ts-node/register --require jsdom-global/register --watch --watch-extensions 'ts?(x)' './src/**/*.test.ts?(x)'",
+    "test:ci": "mocha --require ts-node/register --require jsdom-global/register './src/**/*.test.ts?(x)'",
+    "storybook": "start-storybook -s ./src/extension/assets -c ./config/storybook -p 6006",
+    "build-storybook": "build-storybook -c ./config/storybook"
   },
   "husky": {
     "hooks": {
@@ -87,11 +87,15 @@
     "get-graphql-schema": "^2.1.1",
     "gql2ts": "^1.2.1",
     "husky": "^1.1.0",
+    "jsdom": "^12.2.0",
+    "jsdom-global": "^3.0.2",
     "mocha": "^5.2.0",
     "node-sass": "^4.9.0",
     "postcss-loader": "^3.0.0",
     "prettier": "1.14.0",
+    "raf": "^3.4.0",
     "raw-loader": "^0.5.1",
+    "react-testing-library": "^5.2.1",
     "sass-loader": "^6.0.6",
     "semantic-release": "^15.9.9",
     "semantic-release-chrome": "^1.1.0",
@@ -116,6 +120,7 @@
     "@sourcegraph/extensions-client-common": "^11.0.1",
     "@sourcegraph/react-loading-spinner": "0.0.6",
     "@sqs/jsonc-parser": "^1.0.3",
+    "@types/sinon": "^5.0.5",
     "@types/uglifyjs-webpack-plugin": "1.1.0",
     "bootstrap": "^4.0.0",
     "cypress-browser-extension-plugin": "^0.1.0",
@@ -136,6 +141,7 @@
     "react-router-dom": "^4.3.1",
     "reactstrap": "^5.0.0-beta.2",
     "rxjs": "^6.3.2",
+    "sinon": "^7.1.0",
     "socket.io-client": "^2.1.1",
     "sourcegraph": "^18.4.0",
     "string-score": "^1.0.1",

src/shared/app.scss → src/app.scss

@@ -79,15 +79,16 @@ $theme-colors-light: (
 @import 'bootstrap/scss/popover';
 @import './global-styles/card';
 @import './global-styles/icons';
-@import './repo/tooltips';
+@import './shared/repo/tooltips';
 @import './highlight';
 @import './options';
-@import './components/alerts';
-@import './components/symbols';
-@import './components/codeIntelStatusIndicator';
-@import './components/CodeViewToolbar.scss';
+@import './shared/components/alerts';
+@import './shared/components/symbols';
+@import './shared/components/codeIntelStatusIndicator';
+@import './shared/components/CodeViewToolbar.scss';
+@import './shared/components/JSONEditor';
 @import '@sourcegraph/codeintellify/lib/HoverOverlay.scss';
-@import '../libs/code_intelligence/HoverOverlay.scss';
+@import './libs/code_intelligence/HoverOverlay.scss';
 @import '@sourcegraph/react-loading-spinner/lib/LoadingSpinner.css';
 @import './extensions-client-common';
 

src/browser/runtime.ts

@@ -1,4 +1,4 @@
-import { isBackground } from '../shared/context'
+import { isBackground } from '../context'
 import { getURL } from './extension'
 import safariMessager from './safari/SafariMessager'
 

src/browser/storage.ts

@@ -8,7 +8,13 @@ export { StorageItems, defaultStorageItems } from './types'
 type MigrateFunc = (
     items: StorageItems,
     set: (items: Partial<StorageItems>) => void,
-    remove: (key: keyof StorageItems) => void
+    /**
+     * Remove an item from storage.
+     *
+     * @param key the key of the item you'd like to remove. We accept arbitary
+     * strings so we can remove items that are no longer in our types.
+     */
+    remove: (key: string) => void
 ) => void
 
 export interface Storage {

src/browser/types.ts

@@ -11,13 +11,73 @@ export interface PhabricatorMapping {
  * The feature flags available.
  */
 export interface FeatureFlags {
-    newTooltips: boolean
+    /**
+     * Whether or not to render [Mermaid](https://mermaidjs.github.io/) graphs
+     * in markdown files viewed on GitHub.
+     *
+     * @duration permanent
+     */
+    renderMermaidGraphsEnabled: boolean
+    /**
+     * Open files from the fuzzy file finder omnibar tool (src :f <file_name>)
+     * on Sourcegraph or the codehost.
+     *
+     * @duration permanent
+     */
+    openFileOnSourcegraph: boolean
+    /**
+     * Whether or not to use the new inject method for code intelligence.
+     *
+     * @duration temporary - to be removed November first.
+     */
     newInject: boolean
+    /**
+     * Enable the use of Sourcegraph extensions.
+     *
+     * @duration temporary - to be removed by @chris when extensions are stable and out of
+     * beta.
+     */
+    useExtensions: boolean
+    /**
+     * Enable inline symbol search by typing `!symbolQueryText` inside of GitHub PR comments (requires reload after toggling).
+     *
+     * @duration temporary - needs feedback from users.
+     */
+    inlineSymbolSearchEnabled: boolean
+    /**
+     * Whether or not to execute a search on Sourcegraph when a search is
+     * executed on the code host.
+     *
+     * @duration permanent
+     */
+    executeSearchEnabled: boolean
+    /**
+     * Display the Sourcegraph file tree in the code host when viewing a repository.
+     *
+     * @duration permanent
+     */
+    repositoryFileTreeEnabled: boolean
 }
 
 export const featureFlagDefaults: FeatureFlags = {
-    newTooltips: true,
     newInject: false,
+    renderMermaidGraphsEnabled: false,
+    useExtensions: false,
+    openFileOnSourcegraph: true,
+    inlineSymbolSearchEnabled: true,
+    executeSearchEnabled: false,
+    repositoryFileTreeEnabled: true,
+}
+
+/** A map determining whether a feature flag is configurable by users or not. */
+export const configurableFeatureFlags = {
+    newInject: false,
+    renderMermaidGraphsEnabled: true,
+    useExtensions: true,
+    openFileOnSourcegraph: true,
+    inlineSymbolSearchEnabled: true,
+    executeSearchEnabled: false,
+    repositoryFileTreeEnabled: true,
 }
 
 export interface AccessToken {
@@ -42,24 +102,17 @@ export interface StorageItems {
 
     gitHubEnterpriseURL: string
     phabricatorURL: string
-    inlineSymbolSearchEnabled: boolean
-    renderMermaidGraphsEnabled: boolean
     identity: string
     serverUrls: string[]
     enterpriseUrls: string[]
     serverUserId: string
     hasSeenServerModal: boolean
     repoLocations: RepoLocations
     phabricatorMappings: PhabricatorMapping[]
-    openFileOnSourcegraph: boolean
     sourcegraphAnonymousUid: string
     disableExtension: boolean
     /**
-     * Enable the use of Sourcegraph extensions.
-     */
-    useExtensions: boolean
-    /**
-     * Storage for feature flags
+     * Storage for feature flags.
      */
     featureFlags: FeatureFlags
     clientConfiguration: ClientConfigurationDetails
@@ -83,18 +136,14 @@ export const defaultStorageItems: StorageItems = {
     serverUrls: ['https://sourcegraph.com'],
     gitHubEnterpriseURL: '',
     phabricatorURL: '',
-    inlineSymbolSearchEnabled: true,
-    renderMermaidGraphsEnabled: false,
     identity: '',
     enterpriseUrls: [],
     serverUserId: '',
     hasSeenServerModal: false,
     repoLocations: {},
     phabricatorMappings: [],
-    openFileOnSourcegraph: true,
     sourcegraphAnonymousUid: '',
     disableExtension: false,
-    useExtensions: false,
     featureFlags: featureFlagDefaults,
     clientConfiguration: {
         contentScriptUrls: [],

src/extension/scripts/background.tsx

@@ -13,9 +13,10 @@ import * as permissions from '../../browser/permissions'
 import * as runtime from '../../browser/runtime'
 import storage, { defaultStorageItems } from '../../browser/storage'
 import * as tabs from '../../browser/tabs'
+import { featureFlagDefaults } from '../../browser/types'
 import initializeCli from '../../libs/cli'
+import { ExtensionConnectionInfo, onFirstMessage } from '../../messaging'
 import { resolveClientConfiguration } from '../../shared/backend/server'
-import { ExtensionConnectionInfo, onFirstMessage } from '../../shared/messaging'
 import { DEFAULT_SOURCEGRAPH_URL, setSourcegraphUrl, sourcegraphUrl } from '../../shared/util/context'
 import { assertEnv } from '../envAssertion'
 
@@ -161,6 +162,7 @@ permissions.onRemoved(permissions => {
     })
 })
 
+// Ensure access tokens are in storage and they are in the correct shape.jj
 storage.addSyncMigration((items, set, remove) => {
     if (!items.accessTokens) {
         set({ accessTokens: {} })
@@ -178,50 +180,22 @@ storage.addSyncMigration((items, set, remove) => {
 
         set({ accessTokens })
     }
+})
 
-    if (items.phabricatorURL) {
-        remove('phabricatorURL')
-
-        const newItems: {
-            enterpriseUrls?: string[]
-        } = {}
-
-        if (items.enterpriseUrls && !items.enterpriseUrls.find(u => u === items.phabricatorURL)) {
-            newItems.enterpriseUrls = items.enterpriseUrls.concat(items.phabricatorURL)
-        } else if (!items.enterpriseUrls) {
-            newItems.enterpriseUrls = [items.phabricatorURL]
+// Ensure all feature flags are in storage.
+storage.addSyncMigration((items, set, remove) => {
+    for (const key of Object.keys(featureFlagDefaults)) {
+        if (typeof items.featureFlags[key] === 'undefined') {
+            remove(key)
+            set({ featureFlags: { ...featureFlagDefaults, ...items.featureFlags, [key]: featureFlagDefaults[key] } })
         }
-
-        set(newItems)
-    }
-
-    if (!items.repoLocations) {
-        set({ repoLocations: {} })
-    }
-
-    if (items.openFileOnSourcegraph === undefined) {
-        set({ openFileOnSourcegraph: true })
-    }
-
-    if (items.featureFlags && !items.featureFlags.newInject) {
-        set({ featureFlags: { ...items.featureFlags, newInject: true } })
-    }
-
-    if (!items.inlineSymbolSearchEnabled) {
-        set({ inlineSymbolSearchEnabled: true })
     }
+})
 
-    if (items.serverUrls) {
-        if (items.sourcegraphURL) {
-            if (items.sourcegraphURL === DEFAULT_SOURCEGRAPH_URL) {
-                const urls = without(items.serverUrls, DEFAULT_SOURCEGRAPH_URL)
-                if (urls.length) {
-                    set({ sourcegraphURL: urls[0], serverUrls: [urls[0]] })
-                }
-            } else {
-                set({ serverUrls: [items.sourcegraphURL] })
-            }
-        }
+// Add access tokens to storage.
+storage.addSyncMigration((items, set) => {
+    if (!items.accessTokens) {
+        set({ accessTokens: {} })
     }
 })
 
@@ -371,12 +345,9 @@ function handleManagedPermissionRequest(managedUrls: string[]): void {
 
 function setDefaultBrowserAction(): void {
     browserAction.setBadgeText({ text: '' })
+    browserAction.setPopup({ popup: 'options.html?popup=true' })
 }
 
-browserAction.onClicked(() => {
-    runtime.openOptionsPage()
-})
-
 /**
  * Fetches JavaScript from a URL and runs it in a web worker.
  */

src/extension/scripts/inject.tsx

@@ -74,12 +74,16 @@ function injectApplication(): void {
         if (isGitHub || isGitHubEnterprise) {
             setSourcegraphUrl(sourcegraphServerUrl)
             setRenderMermaidGraphsEnabled(
-                items.renderMermaidGraphsEnabled === undefined ? false : items.renderMermaidGraphsEnabled
+                items.featureFlags.renderMermaidGraphsEnabled === undefined
+                    ? false
+                    : items.featureFlags.renderMermaidGraphsEnabled
             )
             setInlineSymbolSearchEnabled(
-                items.inlineSymbolSearchEnabled === undefined ? false : items.inlineSymbolSearchEnabled
+                items.featureFlags.inlineSymbolSearchEnabled === undefined
+                    ? false
+                    : items.featureFlags.inlineSymbolSearchEnabled
             )
-            injectGitHubApplication(extensionMarker)
+            await injectGitHubApplication(extensionMarker)
         } else if (isSourcegraphServer || /^https?:\/\/(www.)?sourcegraph.com/.test(href)) {
             setSourcegraphUrl(sourcegraphServerUrl)
             injectSourcegraphApp(extensionMarker)
@@ -100,7 +104,7 @@ function injectApplication(): void {
             }
         }
 
-        setUseExtensions(items.useExtensions === undefined ? false : items.useExtensions)
+        setUseExtensions(items.featureFlags.useExtensions === undefined ? false : items.featureFlags.useExtensions)
     }
 
     storage.getSync(handleGetStorage)

src/extension/scripts/options.tsx

@@ -5,7 +5,7 @@ import '../../config/polyfill'
 import * as React from 'react'
 import { render } from 'react-dom'
 import storage from '../../browser/storage'
-import { OptionsDashboard } from '../../shared/components/options/OptionsDashboard'
+import { OptionsContainer } from '../../libs/options/OptionsContainer'
 import { assertEnv } from '../envAssertion'
 
 assertEnv('OPTIONS')
@@ -17,7 +17,7 @@ const inject = () => {
     document.body.appendChild(injectDOM)
 
     storage.getSync(items => {
-        render(<OptionsDashboard />, injectDOM)
+        render(<OptionsContainer />, injectDOM)
     })
 }
 

src/extension/views/options.html

@@ -1,12 +1,12 @@
 <!DOCTYPE html>
-<html style="height: 100%; width: 100%; display: flex;">
+<html>
   <head>
     <meta http-equiv="Content-type" content="text/html; charset=utf-8"/>
     <title>Sourcegraph extension</title>
     <link href="./css/bootstrap.bundle.css" rel="stylesheet" type="text/css">
     <link href="./css/style.bundle.css" rel="stylesheet" type="text/css">
   </head>
-  <body style="height: 100%; width: 100%; display: flex;">
+  <body>
     <script type="text/javascript" src="./js/options.bundle.js"></script>
   </body>
 </html>