From ae5702f54244ed84b173040dd55ca0c450c0b6df Mon Sep 17 00:00:00 2001 From: Joe Farro Date: Sun, 24 Sep 2017 22:38:29 -0400 Subject: [PATCH 1/2] Improve search dropdowns - Sort services and operations operations (case insensitive) - Filter options based on contains instead of starts with --- .../SearchTracePage/SearchDropdownInput.js | 15 +++++---- src/reducers/services.js | 7 ++++ src/utils/regexp-escape.js | 23 +++++++++++++ src/utils/regexp-escape.test.js | 33 +++++++++++++++++++ src/utils/sort.js | 4 +++ src/utils/sort.test.js | 6 ++++ 6 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 src/utils/regexp-escape.js create mode 100644 src/utils/regexp-escape.test.js diff --git a/src/components/SearchTracePage/SearchDropdownInput.js b/src/components/SearchTracePage/SearchDropdownInput.js index 27d094fff9..b0c566d3d1 100644 --- a/src/components/SearchTracePage/SearchDropdownInput.js +++ b/src/components/SearchTracePage/SearchDropdownInput.js @@ -22,6 +22,8 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; import { Dropdown } from 'semantic-ui-react'; +import regexpEscape from '../../utils/regexp-escape'; + /** * We have to wrap the semantic ui component becuase it doesn't perform well * when there are 200+ suggestions. @@ -33,21 +35,22 @@ export default class SearchDropdownInput extends Component { constructor(props) { super(props); this.state = { - items: props.items, currentItems: props.items.slice(0, props.maxResults), }; + this.onSearch = this.onSearch.bind(this); } componentWillReceiveProps(nextProps) { if (this.props.items.map(i => i.text).join(',') !== nextProps.items.map(i => i.text).join(',')) { this.setState({ - items: nextProps.items, currentItems: nextProps.items.slice(0, nextProps.maxResults), }); } } - onSearch(items, v) { - const { maxResults } = this.props; - return this.state.items.filter(i => i.text.startsWith(v)).slice(0, maxResults); + onSearch(_, searchText) { + const { items, maxResults } = this.props; + const rxStr = regexpEscape(searchText); + const rx = new RegExp(rxStr, 'i'); + return items.filter(v => rx.test(v.text)).slice(0, maxResults); } render() { const { input: { value, onChange } } = this.props; @@ -56,7 +59,7 @@ export default class SearchDropdownInput extends Component { this.onSearch(items, v)} + search={this.onSearch} onChange={(e, { value: newValue }) => onChange(newValue)} options={currentItems} selection diff --git a/src/reducers/services.js b/src/reducers/services.js index fabd1fd130..81965cea49 100644 --- a/src/reducers/services.js +++ b/src/reducers/services.js @@ -21,6 +21,7 @@ import { handleActions } from 'redux-actions'; import { fetchServices, fetchServiceOperations as fetchOps } from '../actions/jaeger-api'; +import { baseStringComparator } from '../utils/sort'; const initialState = { services: [], @@ -35,6 +36,9 @@ function fetchStarted(state) { function fetchServicesDone(state, { payload }) { const services = payload.data; + if (Array.isArray(services)) { + services.sort(baseStringComparator); + } return { ...state, services, error: null, loading: false }; } @@ -49,6 +53,9 @@ function fetchOpsStarted(state, { meta: { serviceName } }) { function fetchOpsDone(state, { meta, payload }) { const { data: operations } = payload; + if (Array.isArray(operations)) { + operations.sort(baseStringComparator); + } const operationsForService = { ...state.operationsForService, [meta.serviceName]: operations }; return { ...state, operationsForService }; } diff --git a/src/utils/regexp-escape.js b/src/utils/regexp-escape.js new file mode 100644 index 0000000000..f40e63f840 --- /dev/null +++ b/src/utils/regexp-escape.js @@ -0,0 +1,23 @@ +// Copyright (c) 2017 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +export default function regexpEscape(s) { + return s.replace(/[-/\\^$*+?.()|[\]{}]/g, '\\$&'); +} diff --git a/src/utils/regexp-escape.test.js b/src/utils/regexp-escape.test.js new file mode 100644 index 0000000000..2e4f925fe9 --- /dev/null +++ b/src/utils/regexp-escape.test.js @@ -0,0 +1,33 @@ +// Copyright (c) 2017 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +import regexpEscape from './regexp-escape'; + +describe('regexp-escape', () => { + const chars = '-/\\^$*+?.()|[]{}'.split(''); + chars.forEach(c => { + it(`escapes "${c}" correctly`, () => { + const result = regexpEscape(c); + expect(result.length).toBe(2); + expect(result[0]).toBe('\\'); + expect(result[1]).toBe(c); + }); + }); +}); diff --git a/src/utils/sort.js b/src/utils/sort.js index eba0f304d7..514933bebc 100644 --- a/src/utils/sort.js +++ b/src/utils/sort.js @@ -18,6 +18,10 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. +export function baseStringComparator(itemA, itemB) { + return itemA.localeCompare(itemB, 'en', { sensitivity: 'base' }); +} + export function stringSortComparator(itemA, itemB) { return itemA.localeCompare(itemB); } diff --git a/src/utils/sort.test.js b/src/utils/sort.test.js index 0e275db13a..0dacc9619b 100644 --- a/src/utils/sort.test.js +++ b/src/utils/sort.test.js @@ -22,6 +22,12 @@ import sinon from 'sinon'; import * as sortUtils from './sort'; +it('baseStringComparator() provides a case-insensitive sort', () => { + const arr = ['Z', 'ab', 'AC']; + expect(arr.slice().sort()).toEqual(['AC', 'Z', 'ab']); + expect(arr.slice().sort(sortUtils.baseStringComparator)).toEqual(['ab', 'AC', 'Z']); +}); + it('stringSortComparator() should properly sort a list of strings', () => { const arr = ['allen', 'Gustav', 'paul', 'Tim', 'abernathy', 'tucker', 'Steve', 'mike', 'John', 'Paul']; From 99c2d424af62f7f2fd6273c3369eb6b2c814ac5c Mon Sep 17 00:00:00 2001 From: Joe Farro Date: Tue, 26 Sep 2017 10:57:21 -0400 Subject: [PATCH 2/2] Better var name and added a comment --- src/components/SearchTracePage/SearchDropdownInput.js | 6 +++--- src/utils/regexp-escape.js | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/components/SearchTracePage/SearchDropdownInput.js b/src/components/SearchTracePage/SearchDropdownInput.js index b0c566d3d1..60a080f6e2 100644 --- a/src/components/SearchTracePage/SearchDropdownInput.js +++ b/src/components/SearchTracePage/SearchDropdownInput.js @@ -48,9 +48,9 @@ export default class SearchDropdownInput extends Component { } onSearch(_, searchText) { const { items, maxResults } = this.props; - const rxStr = regexpEscape(searchText); - const rx = new RegExp(rxStr, 'i'); - return items.filter(v => rx.test(v.text)).slice(0, maxResults); + const regexStr = regexpEscape(searchText); + const regex = new RegExp(regexStr, 'i'); + return items.filter(v => regex.test(v.text)).slice(0, maxResults); } render() { const { input: { value, onChange } } = this.props; diff --git a/src/utils/regexp-escape.js b/src/utils/regexp-escape.js index f40e63f840..844b2bf44a 100644 --- a/src/utils/regexp-escape.js +++ b/src/utils/regexp-escape.js @@ -18,6 +18,9 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. +/** + * Escape the meta-caharacters used in regular expressions. + */ export default function regexpEscape(s) { return s.replace(/[-/\\^$*+?.()|[\]{}]/g, '\\$&'); }