From f110114833548253fd6a2e057bbee757af8d7d4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Thu, 26 Jul 2018 19:24:25 -0700 Subject: [PATCH] Convert Explorer repository selection to dropdown (#529) Context: The Cred Explorer loads data (currently on a per-repository basis) that has previously been prepared by running the `sourcecred load` cli command. Currently, to select a repository to load, the user must manually type the repository owner and name. This is a confusing UI, because it suggests that any repository may be chosen, when in fact only repos already loaded into the data store are available. The user is given no feedback as to which repositories are valid options. As of #516, the backend stores a registry listing available repositories. This commit adds a `RepositorySelect` component which loads the available from that registry, and makes them available in a dropdown, in sorted order. When the user manually selects one of the repositories, that selection is persisted into localStorage and respected on future loads. If the user hasn't made such a choice, then the first repository is selected by default. The implementation is highly influenced by testability considerations. The default export, ``, is pretty straightforward. The `RepositorySelect` is somewhat cumbersome to test because it asynchronously fetches data and then updates its state, which affects the render output. So as to avoid testing inside async react components wherever possible, I've factored out: * `loadStatus`, which uses fetch and localStore to get the status of the selector. * `PureRepositorySelect`, which just renders a `Status`, such as loading, failure, or valid * `LocalStoreRepositorySelect`, which wraps the `PureRepositorySelect` with logic to bind the repository select to localStore on change. Test plan: Extensive unit tests were added. Also, to ensure that the tests were testing the right thing, I manually tested: - attempting to load invalid registry - attempting to load with no registry - attempting to load with empty registry - loading without valid localStore - changing the setting via dropdown - loading from localStore after changing the dropdown And all behavior was as expected. Thanks to @wchargin for considerable help testing this PR. --- src/app/credExplorer/App.js | 75 +--- src/app/credExplorer/RepositorySelect.js | 188 +++++++++ src/app/credExplorer/RepositorySelect.test.js | 362 ++++++++++++++++++ 3 files changed, 571 insertions(+), 54 deletions(-) create mode 100644 src/app/credExplorer/RepositorySelect.js create mode 100644 src/app/credExplorer/RepositorySelect.test.js diff --git a/src/app/credExplorer/App.js b/src/app/credExplorer/App.js index 16ba2a7..e4af73b 100644 --- a/src/app/credExplorer/App.js +++ b/src/app/credExplorer/App.js @@ -16,13 +16,10 @@ import type {DynamicPluginAdapter} from "../pluginAdapter"; import {type EdgeEvaluator} from "../../core/attribution/pagerank"; import {WeightConfig} from "./WeightConfig"; import type {PagerankNodeDecomposition} from "../../core/attribution/pagerankNodeDecomposition"; +import RepositorySelect, {type Repo} from "./RepositorySelect"; import * as NullUtil from "../../util/null"; -const REPO_OWNER_KEY = "repoOwner"; -const REPO_NAME_KEY = "repoName"; -const MAX_ENTRIES_PER_LIST = 100; - export default class AppPage extends React.Component<{||}> { static _LOCAL_STORE = new CheckedLocalStore( new BrowserLocalStore({ @@ -38,8 +35,7 @@ export default class AppPage extends React.Component<{||}> { type Props = {|+localStore: LocalStore|}; type State = { - repoOwner: string, - repoName: string, + selectedRepo: ?Repo, data: {| graphWithMetadata: ?{| +graph: Graph, @@ -52,28 +48,21 @@ type State = { edgeEvaluator: ?EdgeEvaluator, }; +const MAX_ENTRIES_PER_LIST = 100; + export class App extends React.Component { constructor(props: Props) { super(props); this.state = { - repoOwner: "", - repoName: "", + selectedRepo: null, data: {graphWithMetadata: null, pnd: null}, edgeEvaluator: null, }; } - componentDidMount() { - const {localStore} = this.props; - this.setState((state) => ({ - repoOwner: localStore.get(REPO_OWNER_KEY, state.repoOwner), - repoName: localStore.get(REPO_NAME_KEY, state.repoName), - })); - } - render() { const {localStore} = this.props; - const {edgeEvaluator} = this.state; + const {edgeEvaluator, selectedRepo} = this.state; const {graphWithMetadata, pnd} = this.state.data; return (
@@ -81,33 +70,17 @@ export class App extends React.Component {

Cred Explorer

- + this.setState({selectedRepo})} + />
- -
- +