Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added <QuerySelector /> component #3494

Merged
merged 5 commits into from
Feb 28, 2019
Merged

Added <QuerySelector /> component #3494

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1a50cab
Updated npm to support react hooks
ranbena Feb 26, 2019
0d209cb
Added <QuerySelector />
ranbena Feb 25, 2019
cd9f1e0
Changed selectQuery to also clear, completed 2->3 dots in msg, avoidi…
ranbena Feb 27, 2019
325be9f
Removed unused highlight lib
ranbena Feb 27, 2019
a5b0aaf
Merge branch 'master' into query-selector
ranbena Feb 28, 2019
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
1 change: 1 addition & 0 deletions client/.eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ module.exports = {
'no-param-reassign': 0,
'no-mixed-operators': 0,
'no-underscore-dangle': 0,
"no-use-before-define": ["error", "nofunc"],
ranbena marked this conversation as resolved.
Show resolved Hide resolved
"prefer-destructuring": "off",
"prefer-template": "off",
"no-restricted-properties": "off",
Expand Down
149 changes: 149 additions & 0 deletions client/app/components/QuerySelector.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import React, { useState, useEffect } from 'react';
import PropTypes from 'prop-types';
import cx from 'classnames';
import { debounce, find } from 'lodash';
import Input from 'antd/lib/input';
import { Query } from '@/services/query';
import { toastr } from '@/services/ng';
import { QueryTagsControl } from '@/components/tags-control/TagsControl';

const SEARCH_DEBOUNCE_DURATION = 200;

function search(term) {
// get recent
if (!term) {
return Query.recent().$promise
.then((results) => {
const filteredResults = results.filter(item => !item.is_draft); // filter out draft
return Promise.resolve(filteredResults);
});
}

// search by query
return Query.query({ q: term }).$promise
.then(({ results }) => Promise.resolve(results));
}

export function QuerySelector(props) {
const [searchTerm, setSearchTerm] = useState();
const [searching, setSearching] = useState();
const [searchResults, setSearchResults] = useState([]);
const [selectedQuery, setSelectedQuery] = useState();

let isStaleSearch = false;
const debouncedSearch = debounce(_search, SEARCH_DEBOUNCE_DURATION);
const placeholder = 'Search a query by name';
const clearIcon = <i className="fa fa-times" onClick={() => setSelectedQuery(null)} />;
const spinIcon = <i className={cx('fa fa-spinner fa-pulse', { hidden: !searching })} />;

// set selected from prop
useEffect(() => {
if (props.selectedQuery) {
setSelectedQuery(props.selectedQuery);
}
arikfr marked this conversation as resolved.
Show resolved Hide resolved
}, [props.selectedQuery]);

// on search term changed, debounced
useEffect(() => {
// clear results, no search
if (searchTerm === null) {
setSearchResults(null);
return () => {};
}

// search
debouncedSearch(searchTerm);
return () => {
debouncedSearch.cancel();
isStaleSearch = true;
};
}, [searchTerm]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ranbena Look: you still load search results as a side-effect of changing search term. It's implicit dependency, that's what makes code hard to read and understand:

$scope.setSearchTerm = (searchTerm) => {
  $scope.searchTerm = searchTerm;
};

// somewhere else

$scope.$watch('searchTerm', (searchTerm) => {
  if (searchTerm == '') {
    $scope.results = [];
  } else {
    doSearch(searchTerm);
  }
});

in opposite to explicit reaction, where you immediately see that if you change search term - it will do some extra stuff:

$scope.setSearchTerm = (searchTerm) => {
  $scope.searchTerm = searchTerm;
  if (searchTerm == '') {
    $scope.results = [];
  } else {
    doSearch(searchTerm);
  }
};

I intentionally provide examples in Angular because this pattern is really common there, and one of best practices in React for a long time was to avoid such code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this useEffect also allows easy stale/debounce management (the alternative I assume is having dedicated refs), I'd rather keep it and keep moving 🏃


// on query selected/cleared
useEffect(() => {
setSearchTerm(selectedQuery ? null : ''); // empty string forces recent fetch
props.onChange(selectedQuery);
}, [selectedQuery]);
arikfr marked this conversation as resolved.
Show resolved Hide resolved

function _search(term) {
setSearching(true);
search(term)
.then(rejectStale)
.then(setSearchResults)
.finally(() => {
setSearching(false);
arikfr marked this conversation as resolved.
Show resolved Hide resolved
});
}

function rejectStale(results) {
return isStaleSearch
? Promise.reject(new Error('stale'))
: Promise.resolve(results);
}

function selectQuery(queryId) {
const query = find(searchResults, { id: queryId });
if (!query) { // shouldn't happen
toastr.error('Something went wrong.. Couldn\'t select query');
kravets-levko marked this conversation as resolved.
Show resolved Hide resolved
}
setSelectedQuery(query);
}

function renderResults() {
if (!searchResults.length) {
return <div className="text-muted">No results matching search term.</div>;
}

return (
<div className="list-group">
{searchResults.map(q => (
<a
href="javascript:void(0)"
className={cx('list-group-item', { inactive: q.is_draft })}
key={q.id}
onClick={() => selectQuery(q.id)}
>
{q.name}
kravets-levko marked this conversation as resolved.
Show resolved Hide resolved
{' '}
<QueryTagsControl isDraft={q.is_draft} tags={q.tags} className="inline-tags-control" />
</a>
))}
</div>
);
}

if (props.disabled) {
return <Input value={selectedQuery && selectedQuery.name} placeholder={placeholder} disabled />;
}

return (
<React.Fragment>
{selectedQuery ? (
<Input value={selectedQuery.name} suffix={clearIcon} readOnly />
) : (
<Input
placeholder={placeholder}
value={searchTerm}
onChange={e => setSearchTerm(e.target.value)}
suffix={spinIcon}
/>
)}
<div className="scrollbox" style={{ maxHeight: '50vh', marginTop: 15 }}>
ranbena marked this conversation as resolved.
Show resolved Hide resolved
{searchResults && renderResults()}
</div>
</React.Fragment>
);
}

QuerySelector.propTypes = {
onChange: PropTypes.func.isRequired,
selectedQuery: PropTypes.object, // eslint-disable-line react/forbid-prop-types
disabled: PropTypes.bool,
};

QuerySelector.defaultProps = {
selectedQuery: null,
disabled: false,
};

export default QuerySelector;
162 changes: 12 additions & 150 deletions client/app/components/dashboards/AddWidgetDialog.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import { debounce, each, values, map, includes, first, identity } from 'lodash';
import { each, values, map, includes, first } from 'lodash';
import React from 'react';
import PropTypes from 'prop-types';
import Select from 'antd/lib/select';
import Modal from 'antd/lib/modal';
import { wrap as wrapDialog, DialogPropType } from '@/components/DialogWrapper';
import { BigMessage } from '@/components/BigMessage';
import highlight from '@/lib/highlight';
import {
MappingType,
ParameterMappingListInput,
editableMappingsToParameterMappings,
synchronizeWidgetTitles,
} from '@/components/ParameterMappingInput';
import { QueryTagsControl } from '@/components/tags-control/TagsControl';
import { QuerySelector } from '@/components/QuerySelector';

import { toastr } from '@/services/ng';
import { Widget } from '@/services/widget';
Expand All @@ -26,51 +24,23 @@ class AddWidgetDialog extends React.Component {
dialog: DialogPropType.isRequired,
};

constructor(props) {
super(props);
this.state = {
saveInProgress: false,
selectedQuery: null,
searchTerm: '',
highlightSearchTerm: false,
recentQueries: [],
queries: [],
selectedVis: null,
parameterMappings: [],
isLoaded: false,
};

const searchQueries = debounce(this.searchQueries.bind(this), 200);
this.onSearchTermChanged = (event) => {
const searchTerm = event.target.value;
this.setState({ searchTerm });
searchQueries(searchTerm);
};
}

componentDidMount() {
Query.recent().$promise.then((items) => {
// Don't show draft (unpublished) queries in recent queries.
const results = items.filter(item => !item.is_draft);
this.setState({
recentQueries: results,
queries: results,
isLoaded: true,
highlightSearchTerm: false,
});
});
}
state = {
saveInProgress: false,
selectedQuery: null,
selectedVis: null,
parameterMappings: [],
};

selectQuery(queryId) {
selectQuery(selectedQuery) {
// Clear previously selected query (if any)
this.setState({
selectedQuery: null,
selectedVis: null,
parameterMappings: [],
});

if (queryId) {
Query.get({ id: queryId }, (query) => {
if (selectedQuery) {
Query.get({ id: selectedQuery.id }, (query) => {
if (query) {
const existingParamNames = map(
this.props.dashboard.getParametersDefs(),
Expand All @@ -96,31 +66,6 @@ class AddWidgetDialog extends React.Component {
}
}

searchQueries(term) {
if (!term || term.length === 0) {
this.setState(prevState => ({
queries: prevState.recentQueries,
isLoaded: true,
highlightSearchTerm: false,
}));
return;
}

Query.query({ q: term }, (results) => {
// If user will type too quick - it's possible that there will be
// several requests running simultaneously. So we need to check
// which results are matching current search term and ignore
// outdated results.
if (this.state.searchTerm === term) {
this.setState({
queries: results.results,
isLoaded: true,
highlightSearchTerm: true,
});
}
});
}

selectVisualization(query, visualizationId) {
each(query.visualizations, (visualization) => {
if (visualization.id === visualizationId) {
Expand Down Expand Up @@ -173,88 +118,6 @@ class AddWidgetDialog extends React.Component {
this.setState({ parameterMappings });
}

renderQueryInput() {
return (
<div className="form-group">
{!this.state.selectedQuery && (
<input
type="text"
placeholder="Search a query by name"
className="form-control"
value={this.state.searchTerm}
onChange={this.onSearchTermChanged}
/>
)}
{this.state.selectedQuery && (
<div className="p-relative">
<input type="text" className="form-control bg-white" value={this.state.selectedQuery.name} readOnly />
<a
href="javascript:void(0)"
onClick={() => this.selectQuery(null)}
className="d-flex align-items-center justify-content-center"
style={{
position: 'absolute',
right: '1px',
top: '1px',
bottom: '1px',
width: '30px',
background: '#fff',
borderRadius: '3px',
}}
>
<i className="text-muted fa fa-times" />
</a>
</div>
)}
</div>
);
}

renderSearchQueryResults() {
const { isLoaded, queries, highlightSearchTerm, searchTerm } = this.state;

const highlightSearchResult = highlightSearchTerm ? highlight : identity;

return (
<div className="scrollbox" style={{ maxHeight: '50vh' }}>
{!isLoaded && (
<div className="text-center">
<BigMessage icon="fa-spinner fa-2x fa-pulse" message="Loading..." />
</div>
)}

{isLoaded && (
<div>
{
(queries.length === 0) &&
<div className="text-muted">No results matching search term.</div>
}
{(queries.length > 0) && (
<div className="list-group">
{queries.map(query => (
<a
href="javascript:void(0)"
className={'list-group-item ' + (query.is_draft ? 'inactive' : '')}
key={query.id}
onClick={() => this.selectQuery(query.id)}
>
<div
// eslint-disable-next-line react/no-danger
dangerouslySetInnerHTML={{ __html: highlightSearchResult(query.name, searchTerm) }}
style={{ display: 'inline-block' }}
/>
{' '}
<QueryTagsControl isDraft={query.is_draft} tags={query.tags} className="inline-tags-control" />
</a>
))}
</div>
)}
</div>
)}
</div>
);
}

renderVisualizationInput() {
let visualizationGroups = {};
if (this.state.selectedQuery) {
Expand Down Expand Up @@ -304,8 +167,7 @@ class AddWidgetDialog extends React.Component {
okText="Add to Dashboard"
width={700}
>
{this.renderQueryInput()}
{!this.state.selectedQuery && this.renderSearchQueryResults()}
<QuerySelector onChange={query => this.selectQuery(query)} />
{this.state.selectedQuery && this.renderVisualizationInput()}

{
Expand Down
Loading