From 749571ebe998f75334ac189e27bf8898d26e457d Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Wed, 4 May 2016 20:09:53 +0200 Subject: [PATCH] Review feedback * Fix node-details-test for search * Label spacing and matched text truncation * Delete pinned search on backspace, add hint for metrics, escape % in URL * Fix text-bg on node highlight * Added tests for search-utils * Fix matching of other topologies, added comment re quick clear * s/cx/classnames/ * Ignore MoC keys when search in focus, blur on Esc * Fixes search term highlighting on-hover * Fix SVG exports * Fine-tuned search item rendering * Fixed search highlighting in the details panel * Dont throb node on hover * Hotkey for search: '/' * Keep focus on search when tabbing away from the browser * bring hovered node to top * background for search results on hover * fixed height for foreign object to prevent layout glitches * Dont blur focused nodes on search * More robust metric matchers * More meaningful search hints --- client/app/scripts/actions/app-actions.js | 28 +- client/app/scripts/charts/node.js | 109 ++++--- .../app/scripts/charts/nodes-chart-nodes.js | 10 +- client/app/scripts/charts/nodes-layout.js | 2 +- .../components/__tests__/node-details-test.js | 2 +- client/app/scripts/components/app.js | 42 ++- client/app/scripts/components/footer.js | 2 +- client/app/scripts/components/help-panel.js | 2 +- .../app/scripts/components/matched-results.js | 11 +- client/app/scripts/components/matched-text.js | 107 +++++-- client/app/scripts/components/node-details.js | 2 +- .../node-details/node-details-info.js | 5 +- .../node-details/node-details-labels.js | 2 +- client/app/scripts/components/search.js | 74 +++-- client/app/scripts/components/topologies.js | 6 +- client/app/scripts/reducers/root.js | 7 +- .../utils/__tests__/search-utils-test.js | 300 ++++++++++++++++++ client/app/scripts/utils/router-utils.js | 11 +- client/app/scripts/utils/search-utils.js | 71 ++++- client/app/styles/contrast.less | 3 + client/app/styles/main.less | 78 +++-- 21 files changed, 676 insertions(+), 198 deletions(-) create mode 100644 client/app/scripts/utils/__tests__/search-utils-test.js diff --git a/client/app/scripts/actions/app-actions.js b/client/app/scripts/actions/app-actions.js index 3b05f13b3..85a6246eb 100644 --- a/client/app/scripts/actions/app-actions.js +++ b/client/app/scripts/actions/app-actions.js @@ -143,7 +143,9 @@ export function clickCloseTerminal(pipeId, closePipe) { } export function clickDownloadGraph() { - saveGraph(); + return () => { + saveGraph(); + }; } export function clickForceRelayout() { @@ -315,6 +317,23 @@ export function focusSearch() { }; } +export function hitBackspace() { + return (dispatch, getState) => { + const state = getState(); + // remove last pinned query if search query is empty + if (state.get('searchFocused') && !state.get('searchQuery')) { + const query = state.get('pinnedSearches').last(); + if (query) { + dispatch({ + type: ActionTypes.UNPIN_SEARCH, + query + }); + updateRoute(getState); + } + } + }; +} + export function hitEnter() { return (dispatch, getState) => { const state = getState(); @@ -340,6 +359,8 @@ export function hitEsc() { dispatch(hideHelp()); } else if (state.get('searchQuery')) { dispatch(doSearch('')); + } else if (state.get('searchFocused')) { + dispatch(blurSearch()); } else if (controlPipe && controlPipe.get('status') === 'PIPE_DELETED') { dispatch({ type: ActionTypes.CLICK_CLOSE_TERMINAL, @@ -416,6 +437,7 @@ export function receiveNodesForTopology(nodes, topologyId) { export function receiveTopologies(topologies) { return (dispatch, getState) => { + const firstLoad = !getState().get('topologiesLoaded'); dispatch({ type: ActionTypes.RECEIVE_TOPOLOGIES, topologies @@ -431,6 +453,10 @@ export function receiveTopologies(topologies) { state.get('nodeDetails'), dispatch ); + // populate search matches on first load + if (firstLoad && state.get('searchQuery')) { + dispatch(focusSearch()); + } }; } diff --git a/client/app/scripts/charts/node.js b/client/app/scripts/charts/node.js index 1c8ecda43..76c517b7d 100644 --- a/client/app/scripts/charts/node.js +++ b/client/app/scripts/charts/node.js @@ -1,7 +1,8 @@ import React from 'react'; import ReactDOM from 'react-dom'; import { connect } from 'react-redux'; -import classNames from 'classnames'; +import classnames from 'classnames'; +import { Map as makeMap } from 'immutable'; import { clickNode, enterNode, leaveNode } from '../actions/app-actions'; import { getNodeColor } from '../utils/color-utils'; @@ -36,16 +37,6 @@ function getNodeShape({ shape, stack }) { return stack ? stackedShape(nodeShape) : nodeShape; } -function ellipsis(text, fontSize, maxWidth) { - const averageCharLength = fontSize / 1.5; - const allowedChars = maxWidth / averageCharLength; - let truncatedText = text; - if (text && text.length > allowedChars) { - truncatedText = `${text.slice(0, allowedChars)}...`; - } - return truncatedText; -} - class Node extends React.Component { constructor(props, context) { @@ -53,66 +44,80 @@ class Node extends React.Component { this.handleMouseClick = this.handleMouseClick.bind(this); this.handleMouseEnter = this.handleMouseEnter.bind(this); this.handleMouseLeave = this.handleMouseLeave.bind(this); - this.state = { hovered: false }; + this.state = { + hovered: false, + matched: false + }; + } + + componentWillReceiveProps(nextProps) { + // marks as matched only when search query changes + if (nextProps.searchQuery !== this.props.searchQuery) { + this.setState({ + matched: nextProps.matched + }); + } else { + this.setState({ + matched: false + }); + } } render() { - const { blurred, focused, highlighted, label, matched, matches, pseudo, rank, - subLabel, scaleFactor, transform, zoomScale } = this.props; - const { hovered } = this.state; + const { blurred, focused, highlighted, label, matches = makeMap(), + pseudo, rank, subLabel, scaleFactor, transform, zoomScale } = this.props; + const { hovered, matched } = this.state; const nodeScale = focused ? this.props.selectedNodeScale : this.props.nodeScale; const color = getNodeColor(rank, label, pseudo); const truncate = !focused && !hovered; - const labelText = truncate ? ellipsis(label, 14, nodeScale(4 * scaleFactor)) : label; - const subLabelText = truncate ? ellipsis(subLabel, 12, nodeScale(4 * scaleFactor)) : subLabel; + const labelTransform = focused ? `scale(${1 / zoomScale})` : ''; + const labelWidth = nodeScale(scaleFactor * 4); + const labelOffsetX = -labelWidth / 2; + const labelOffsetY = focused ? nodeScale(0.5) : nodeScale(0.5 * scaleFactor); - let labelOffsetY = 8; - let labelFontSize = 14; - let subLabelFontSize = 12; - - // render focused nodes in normal size - if (focused) { - labelFontSize /= zoomScale; - subLabelFontSize /= zoomScale; - labelOffsetY /= zoomScale; - } - - const className = classNames({ - node: true, + const nodeClassName = classnames('node', { highlighted, - blurred, + blurred: blurred && !focused, hovered, matched, pseudo }); + const labelClassName = classnames('node-label', { truncate }); + const subLabelClassName = classnames('node-sublabel', { truncate }); + const NodeShapeType = getNodeShape(this.props); return ( - - - -
- + {/* For browser */} + +
+
+ +
+
+ +
+ {!blurred && }
-
- -
- - + {/* For SVG export */} + + {label} + + {subLabel} + + + + + ); } @@ -135,6 +140,6 @@ class Node extends React.Component { } export default connect( - null, + state => ({ searchQuery: state.get('searchQuery') }), { clickNode, enterNode, leaveNode } )(Node); diff --git a/client/app/scripts/charts/nodes-chart-nodes.js b/client/app/scripts/charts/nodes-chart-nodes.js index 1a7a6bc49..878e18206 100644 --- a/client/app/scripts/charts/nodes-chart-nodes.js +++ b/client/app/scripts/charts/nodes-chart-nodes.js @@ -7,8 +7,8 @@ import NodeContainer from './node-container'; class NodesChartNodes extends React.Component { render() { - const { adjacentNodes, highlightedNodeIds, layoutNodes, - layoutPrecision, nodeScale, scale, searchNodeMatches = makeMap(), + const { adjacentNodes, highlightedNodeIds, layoutNodes, layoutPrecision, + mouseOverNodeId, nodeScale, scale, searchNodeMatches = makeMap(), searchQuery, selectedMetric, selectedNodeScale, selectedNodeId, topCardNode } = this.props; @@ -26,7 +26,10 @@ class NodesChartNodes extends React.Component { // make sure blurred nodes are in the background const sortNodes = node => { - if (node.get('blurred')) { + if (node.get('id') === mouseOverNodeId) { + return 3; + } + if (node.get('blurred') && !node.get('focused')) { return 0; } if (node.get('highlighted')) { @@ -84,6 +87,7 @@ function mapStateToProps(state) { return { adjacentNodes: getAdjacentNodes(state), highlightedNodeIds: state.get('highlightedNodeIds'), + mouseOverNodeId: state.get('mouseOverNodeId'), selectedMetric: state.get('selectedMetric'), selectedNodeId: state.get('selectedNodeId'), searchNodeMatches: state.getIn(['searchNodeMatches', currentTopologyId]), diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 24503f496..f700dd477 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -12,7 +12,7 @@ const DEFAULT_WIDTH = 800; const DEFAULT_MARGINS = {top: 0, left: 0}; const DEFAULT_SCALE = val => val * 2; const NODE_SIZE_FACTOR = 1; -const NODE_SEPARATION_FACTOR = 2.5; +const NODE_SEPARATION_FACTOR = 3.0; const RANK_SEPARATION_FACTOR = 2.5; let layoutRuns = 0; let layoutRunsTrivial = 0; diff --git a/client/app/scripts/components/__tests__/node-details-test.js b/client/app/scripts/components/__tests__/node-details-test.js index e6be1701c..fa97047d3 100644 --- a/client/app/scripts/components/__tests__/node-details-test.js +++ b/client/app/scripts/components/__tests__/node-details-test.js @@ -39,6 +39,6 @@ describe('NodeDetails', () => { nodeId={nodeId} details={details} />); const title = TestUtils.findRenderedDOMComponentWithClass(c, 'node-details-header-label'); - expect(title.textContent).toBe('Node 1'); + expect(title.title).toBe('Node 1'); }); }); diff --git a/client/app/scripts/components/app.js b/client/app/scripts/components/app.js index 271b07672..32bb23cf7 100644 --- a/client/app/scripts/components/app.js +++ b/client/app/scripts/components/app.js @@ -11,7 +11,7 @@ import Status from './status.js'; import Topologies from './topologies.js'; import TopologyOptions from './topology-options.js'; import { getApiDetails, getTopologies } from '../utils/web-api-utils'; -import { pinNextMetric, hitEnter, hitEsc, unpinMetric, +import { focusSearch, pinNextMetric, hitBackspace, hitEnter, hitEsc, unpinMetric, selectMetric, toggleHelp } from '../actions/app-actions'; import Details from './details'; import Nodes from './nodes'; @@ -23,6 +23,7 @@ import DebugToolbar, { showingDebugToolbar, import { getUrlState } from '../utils/router-utils'; import { getActiveTopologyOptions } from '../utils/topology-utils'; +const BACKSPACE_KEY_CODE = 8; const ENTER_KEY_CODE = 13; const ESC_KEY_CODE = 27; const keyPressLog = debug('scope:app-key-press'); @@ -58,31 +59,38 @@ class App extends React.Component { this.props.dispatch(hitEsc()); } else if (ev.keyCode === ENTER_KEY_CODE) { this.props.dispatch(hitEnter()); + } else if (ev.keyCode === BACKSPACE_KEY_CODE) { + this.props.dispatch(hitBackspace()); + } else if (ev.code === 'KeyD' && ev.ctrlKey) { + toggleDebugToolbar(); + this.forceUpdate(); } } onKeyPress(ev) { - const { dispatch } = this.props; + const { dispatch, searchFocused } = this.props; // // keyup gives 'key' // keypress gives 'char' // Distinction is important for international keyboard layouts where there // is often a different {key: char} mapping. // - keyPressLog('onKeyPress', 'keyCode', ev.keyCode, ev); - const char = String.fromCharCode(ev.charCode); - if (char === '<') { - dispatch(pinNextMetric(-1)); - } else if (char === '>') { - dispatch(pinNextMetric(1)); - } else if (char === 'q') { - dispatch(unpinMetric()); - dispatch(selectMetric(null)); - } else if (ev.code === 'KeyD' && ev.ctrlKey) { - toggleDebugToolbar(); - this.forceUpdate(); - } else if (char === '?') { - dispatch(toggleHelp()); + if (!searchFocused) { + keyPressLog('onKeyPress', 'keyCode', ev.keyCode, ev); + const char = String.fromCharCode(ev.charCode); + if (char === '<') { + dispatch(pinNextMetric(-1)); + } else if (char === '>') { + dispatch(pinNextMetric(1)); + } else if (char === 'q') { + dispatch(unpinMetric()); + dispatch(selectMetric(null)); + } else if (char === '/') { + ev.preventDefault(); + dispatch(focusSearch()); + } else if (char === '?') { + dispatch(toggleHelp()); + } } } @@ -133,6 +141,8 @@ function mapStateToProps(state) { controlPipes: state.get('controlPipes'), nodeDetails: state.get('nodeDetails'), routeSet: state.get('routeSet'), + searchFocused: state.get('searchFocused'), + searchQuery: state.get('searchQuery'), showingHelp: state.get('showingHelp'), urlState: getUrlState(state) }; diff --git a/client/app/scripts/components/footer.js b/client/app/scripts/components/footer.js index 6b5b37aee..fe9a45ed4 100644 --- a/client/app/scripts/components/footer.js +++ b/client/app/scripts/components/footer.js @@ -72,7 +72,7 @@ class Footer extends React.Component { + title="Save canvas as SVG (does not include search highlighting)"> diff --git a/client/app/scripts/components/help-panel.js b/client/app/scripts/components/help-panel.js index 42564a249..9df27eb92 100644 --- a/client/app/scripts/components/help-panel.js +++ b/client/app/scripts/components/help-panel.js @@ -2,6 +2,7 @@ import React from 'react'; const GENERAL_SHORTCUTS = [ {key: 'esc', label: 'Close active panel'}, + {key: '/', label: 'Activate search field'}, {key: '?', label: 'Toggle shortcut menu'}, ]; @@ -41,4 +42,3 @@ export default class HelpPanel extends React.Component { ); } } - diff --git a/client/app/scripts/components/matched-results.js b/client/app/scripts/components/matched-results.js index d860a7e6f..e044b69f5 100644 --- a/client/app/scripts/components/matched-results.js +++ b/client/app/scripts/components/matched-results.js @@ -3,19 +3,22 @@ import { connect } from 'react-redux'; import MatchedText from './matched-text'; -const SHOW_ROW_COUNT = 3; +const SHOW_ROW_COUNT = 2; +const MAX_MATCH_LENGTH = 24; class MatchedResults extends React.Component { renderMatch(matches, field) { const match = matches.get(field); + const text = match.text; + return (
{match.label}: - +
); @@ -41,9 +44,9 @@ class MatchedResults extends React.Component { return (
{matches.keySeq().take(SHOW_ROW_COUNT).map(fieldId => this.renderMatch(matches, fieldId))} - {moreFieldMatches && + {moreFieldMatches &&
{`${moreFieldMatches.size} more matches`} - } +
}
); } diff --git a/client/app/scripts/components/matched-text.js b/client/app/scripts/components/matched-text.js index a981a992b..2b00f29b9 100644 --- a/client/app/scripts/components/matched-text.js +++ b/client/app/scripts/components/matched-text.js @@ -1,67 +1,104 @@ import React from 'react'; import { connect } from 'react-redux'; +const TRUNCATE_CONTEXT = 6; +const TRUNCATE_ELLIPSIS = '…'; + /** * Returns an array with chunks that cover the whole text via {start, length} * objects. * - * `([{start: 2, length: 1}], "text") => - * [{start: 0, length: 2}, {start: 2, length: 1, match: true}, {start: 3, length: 1}]` + * `('text', {start: 2, length: 1}) => [{text: 'te'}, {text: 'x', match: true}, {text: 't'}]` */ -function reduceMatchesToChunks(matches, text) { - if (text && matches && matches.length > 0) { - const result = matches.reduce((chunks, match) => { - const prev = chunks.length > 0 ? chunks[chunks.length - 1] : null; - const end = prev ? prev.start + prev.length : 0; - // skip non-matching chunk if first chunk is match - if (match.start > 0) { - chunks.push({start: end, length: match.start}); - } - chunks.push(Object.assign({match: true}, match)); - return chunks; - }, []); - const last = result[result.length - 1]; - const remaining = last.start + last.length; - if (text && remaining < text.length) { - result.push({start: remaining, length: text.length - remaining}); +function chunkText(text, { start, length }) { + if (text && !isNaN(start) && !isNaN(length)) { + const chunks = []; + // text chunk before match + if (start > 0) { + chunks.push({text: text.substr(0, start)}); } - return result; + // matching chunk + chunks.push({match: true, text: text.substr(start, length)}); + // text after match + const remaining = start + length; + if (remaining < text.length) { + chunks.push({text: text.substr(remaining)}); + } + return chunks; } - return []; + return [{ text }]; } /** - * Renders text with highlighted search matches. + * Truncates chunks with ellipsis * - * `props.matches` must be an immutable.Map of match - * objects, the match object for this component will be extracted - * via `get(props.fieldId)`). - * A match object is of shape `{text, label, matches}`. - * `match.matches` is an array of text matches of shape `{start, length}` + * First chunk is truncated from left, second chunk (match) is truncated in the + * middle, last chunk is truncated at the end, e.g. + * `[{text: "...cation is a "}, {text: "useful...or not"}, {text: "tool..."}]` + */ +function truncateChunks(chunks, text, maxLength) { + if (chunks && chunks.length === 3 && maxLength && text && text.length > maxLength) { + const res = chunks.map(c => Object.assign({}, c)); + let needToCut = text.length - maxLength; + // trucate end + const end = res[2]; + if (end.text.length > TRUNCATE_CONTEXT) { + needToCut -= end.text.length - TRUNCATE_CONTEXT; + end.text = `${end.text.substr(0, TRUNCATE_CONTEXT)}${TRUNCATE_ELLIPSIS}`; + } + + if (needToCut) { + // truncate front + const start = res[0]; + if (start.text.length > TRUNCATE_CONTEXT) { + needToCut -= start.text.length - TRUNCATE_CONTEXT; + start.text = `${TRUNCATE_ELLIPSIS}` + + `${start.text.substr(start.text.length - TRUNCATE_CONTEXT)}`; + } + } + + if (needToCut) { + // truncate match + const middle = res[1]; + if (middle.text.length > 2 * TRUNCATE_CONTEXT) { + middle.text = `${middle.text.substr(0, TRUNCATE_CONTEXT)}` + + `${TRUNCATE_ELLIPSIS}` + + `${middle.text.substr(middle.text.length - TRUNCATE_CONTEXT)}`; + } + } + + return res; + } + return chunks; +} + +/** + * Renders text with highlighted search match. + * + * A match object is of shape `{text, label, match}`. + * `match` is a text match object of shape `{start, length}` * that delimit text matches in `text`. `label` shows the origin of the text. */ class MatchedText extends React.Component { render() { - const { fieldId, matches, text } = this.props; - // match is a direct match object, or still need to extract the correct field - const fieldMatches = matches && matches.get(fieldId); + const { match, text, maxLength } = this.props; - if (!fieldMatches) { + if (!match) { return {text}; } return ( - - {reduceMatchesToChunks(fieldMatches.matches, text).map((chunk, index) => { + + {truncateChunks(chunkText(text, match), text, maxLength).map((chunk, index) => { if (chunk.match) { return ( - - {text.substr(chunk.start, chunk.length)} + + {chunk.text} ); } - return text.substr(chunk.start, chunk.length); + return chunk.text; })} ); diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index 6ca113c6a..c0e8363c2 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -162,7 +162,7 @@ export class NodeDetails extends React.Component {

- +

{details.parents && } diff --git a/client/app/scripts/components/node-details/node-details-info.js b/client/app/scripts/components/node-details/node-details-info.js index b41e850dd..5804c0904 100644 --- a/client/app/scripts/components/node-details/node-details-info.js +++ b/client/app/scripts/components/node-details/node-details-info.js @@ -1,4 +1,5 @@ import React from 'react'; +import { Map as makeMap } from 'immutable'; import MatchedText from '../matched-text'; import ShowMore from '../show-more'; @@ -19,7 +20,7 @@ export default class NodeDetailsInfo extends React.Component { } render() { - const { matches } = this.props; + const { matches = makeMap() } = this.props; let rows = (this.props.rows || []); let notShown = 0; @@ -41,7 +42,7 @@ export default class NodeDetailsInfo extends React.Component { {field.label}
- +
))} diff --git a/client/app/scripts/components/node-details/node-details-labels.js b/client/app/scripts/components/node-details/node-details-labels.js index 198aa49d2..1832395cd 100644 --- a/client/app/scripts/components/node-details/node-details-labels.js +++ b/client/app/scripts/components/node-details/node-details-labels.js @@ -43,7 +43,7 @@ export default class NodeDetailsLabels extends React.Component { {field.label}
- +
))} diff --git a/client/app/scripts/components/search.js b/client/app/scripts/components/search.js index 832dd9352..688325946 100644 --- a/client/app/scripts/components/search.js +++ b/client/app/scripts/components/search.js @@ -1,6 +1,7 @@ import React from 'react'; +import ReactDOM from 'react-dom'; import { connect } from 'react-redux'; -import cx from 'classnames'; +import classnames from 'classnames'; import _ from 'lodash'; import { blurSearch, doSearch, focusSearch } from '../actions/app-actions'; @@ -8,29 +9,32 @@ import { slugify } from '../utils/string-utils'; import { isTopologyEmpty } from '../utils/topology-utils'; import SearchItem from './search-item'; +function shortenHintLabel(text) { + return text + .split(' ')[0] + .toLowerCase() + .substr(0, 12); +} + // dynamic hint based on node names function getHint(nodes) { let label = 'mycontainer'; let metadataLabel = 'ip'; - let metadataValue = '172.12'; + let metadataValue = '10.1.0.1'; - const node = nodes.last(); + const node = nodes.filter(n => !n.get('pseudo') && n.has('metadata')).last(); if (node) { - label = node.get('label'); + label = shortenHintLabel(node.get('label')) + .split('.')[0]; if (node.get('metadata')) { const metadataField = node.get('metadata').first(); - metadataLabel = slugify(metadataField.get('label')) - .split(' ')[0] - .split('.').pop() - .substr(0, 20); - metadataValue = metadataField.get('value') - .toLowerCase() - .split(' ')[0] - .substr(0, 12); + metadataLabel = shortenHintLabel(slugify(metadataField.get('label'))) + .split('.').pop(); + metadataValue = shortenHintLabel(metadataField.get('value')); } } - return `Try "${label}" or "${metadataLabel}:${metadataValue}". + return `Try "${label}", "${metadataLabel}:${metadataValue}", or "cpu > 2%". Hit enter to apply the search as a filter.`; } @@ -38,7 +42,6 @@ class Search extends React.Component { constructor(props, context) { super(props, context); - this.handleBlur = this.handleBlur.bind(this); this.handleChange = this.handleChange.bind(this); this.handleFocus = this.handleFocus.bind(this); this.doSearch = _.debounce(this.doSearch.bind(this), 200); @@ -47,14 +50,19 @@ class Search extends React.Component { }; } - handleBlur() { - this.props.blurSearch(); - } - handleChange(ev) { - const value = ev.target.value; + const inputValue = ev.target.value; + let value = inputValue; + // In render() props.searchQuery can be set from the outside, but state.value + // must have precendence for quick feedback. Now when the user backspaces + // quickly enough from `text`, a previouse doSearch(`text`) will come back + // via props and override the empty state.value. To detect this edge case + // we instead set value to null when backspacing. + if (this.state.value && value === '') { + value = null; + } this.setState({value}); - this.doSearch(value); + this.doSearch(inputValue); } handleFocus() { @@ -72,15 +80,25 @@ class Search extends React.Component { } } + componentDidUpdate() { + if (this.props.searchFocused) { + ReactDOM.findDOMNode(this.refs.queryInput).focus(); + } else if (!this.state.value) { + ReactDOM.findDOMNode(this.refs.queryInput).blur(); + } + } + render() { const { inputId = 'search', nodes, pinnedSearches, searchFocused, searchNodeMatches, searchQuery, topologiesLoaded } = this.props; - const disabled = this.props.isTopologyEmpty || !topologiesLoaded; + const disabled = this.props.isTopologyEmpty; const matchCount = searchNodeMatches .reduce((count, topologyMatches) => count + topologyMatches.size, 0); const showPinnedSearches = pinnedSearches.size > 0; - const value = this.state.value || searchQuery || ''; - const classNames = cx('search', { + // manual clear (null) has priority, then props, then state + const value = this.state.value === null ? '' : this.state.value || searchQuery || ''; + const classNames = classnames('search', 'hideable', { + hide: !topologiesLoaded, 'search-pinned': showPinnedSearches, 'search-matched': matchCount, 'search-filled': value, @@ -97,14 +115,12 @@ class Search extends React.Component { - {showPinnedSearches && - {pinnedSearches.toIndexedSeq() - .map(query => )} - } + {showPinnedSearches && pinnedSearches.toIndexedSeq() + .map(query => )} + onFocus={this.handleFocus} + disabled={disabled} ref="queryInput" /> {!showPinnedSearches &&
{getHint(nodes)} diff --git a/client/app/scripts/components/topologies.js b/client/app/scripts/components/topologies.js index ea631ba63..8ffcc77aa 100644 --- a/client/app/scripts/components/topologies.js +++ b/client/app/scripts/components/topologies.js @@ -1,6 +1,6 @@ import React from 'react'; import { connect } from 'react-redux'; -import cx from 'classnames'; +import classnames from 'classnames'; import { clickTopology } from '../actions/app-actions'; @@ -22,7 +22,7 @@ class Topologies extends React.Component { const searchMatches = this.props.searchNodeMatches.get(subTopology.get('id')); const searchMatchCount = searchMatches ? searchMatches.size : 0; const title = this.renderTitle(subTopology, searchMatchCount); - const className = cx('topologies-sub-item', { + const className = classnames('topologies-sub-item', { 'topologies-sub-item-active': isActive, 'topologies-sub-item-matched': searchMatchCount }); @@ -50,7 +50,7 @@ class Topologies extends React.Component { const isActive = topology === this.props.currentTopology; const searchMatches = this.props.searchNodeMatches.get(topology.get('id')); const searchMatchCount = searchMatches ? searchMatches.size : 0; - const className = cx('topologies-item-main', { + const className = classnames('topologies-item-main', { 'topologies-item-main-active': isActive, 'topologies-item-main-matched': searchMatchCount }); diff --git a/client/app/scripts/reducers/root.js b/client/app/scripts/reducers/root.js index 2a7eb1a71..3f0349802 100644 --- a/client/app/scripts/reducers/root.js +++ b/client/app/scripts/reducers/root.js @@ -340,6 +340,8 @@ export function rootReducer(state = initialState, action) { const nodeId = action.nodeId; const adjacentNodes = getAdjacentNodes(state, nodeId); + state = state.set('mouseOverNodeId', nodeId); + // highlight adjacent nodes state = state.update('highlightedNodeIds', highlightedNodeIds => { highlightedNodeIds = highlightedNodeIds.clear(); @@ -370,6 +372,7 @@ export function rootReducer(state = initialState, action) { } case ActionTypes.LEAVE_NODE: { + state = state.set('mouseOverNodeId', null); state = state.update('highlightedEdgeIds', highlightedEdgeIds => highlightedEdgeIds.clear()); state = state.update('highlightedNodeIds', highlightedNodeIds => highlightedNodeIds.clear()); return state; @@ -517,7 +520,9 @@ export function rootReducer(state = initialState, action) { case ActionTypes.RECEIVE_NODES_FOR_TOPOLOGY: { // not sure if mergeDeep() brings any benefit here - return state.setIn(['nodesByTopology', action.topologyId], fromJS(action.nodes)); + state = state.setIn(['nodesByTopology', action.topologyId], fromJS(action.nodes)); + state = updateNodeMatches(state); + return state; } case ActionTypes.RECEIVE_NOT_FOUND: { diff --git a/client/app/scripts/utils/__tests__/search-utils-test.js b/client/app/scripts/utils/__tests__/search-utils-test.js new file mode 100644 index 000000000..c9d20dfbf --- /dev/null +++ b/client/app/scripts/utils/__tests__/search-utils-test.js @@ -0,0 +1,300 @@ +jest.dontMock('../search-utils'); +jest.dontMock('../string-utils'); +jest.dontMock('../../constants/naming'); // edge naming: 'source-target' + +import { fromJS } from 'immutable'; + +const SearchUtils = require('../search-utils').testable; + +describe('SearchUtils', () => { + const nodeSets = { + someNodes: fromJS({ + n1: { + id: 'n1', + label: 'node label 1', + metadata: [{ + id: 'fieldId1', + label: 'Label 1', + value: 'value 1' + }], + metrics: [{ + id: 'metric1', + label: 'Metric 1', + value: 1 + }] + }, + n2: { + id: 'n2', + label: 'node label 2', + metadata: [{ + id: 'fieldId2', + label: 'Label 2', + value: 'value 2' + }], + tables: [{ + id: 'metric1', + rows: [{ + id: 'row1', + label: 'Row 1', + value: 'Row Value 1' + }] + }], + }, + }) + }; + + describe('applyPinnedSearches', () => { + const fun = SearchUtils.applyPinnedSearches; + + it('should not filter anything when no pinned searches present', () => { + let nextState = fromJS({ + nodes: nodeSets.someNodes, + pinnedSearches: [] + }); + nextState = fun(nextState); + expect(nextState.get('nodes').filter(node => node.get('filtered')).size).toEqual(0); + }); + + it('should filter nodes if nothing matches a pinned search', () => { + let nextState = fromJS({ + nodes: nodeSets.someNodes, + pinnedSearches: ['cantmatch'] + }); + nextState = fun(nextState); + expect(nextState.get('nodes').filterNot(node => node.get('filtered')).size).toEqual(0); + }); + + it('should filter nodes if nothing matches a combination of pinned searches', () => { + let nextState = fromJS({ + nodes: nodeSets.someNodes, + pinnedSearches: ['node label 1', 'node label 2'] + }); + nextState = fun(nextState); + expect(nextState.get('nodes').filterNot(node => node.get('filtered')).size).toEqual(0); + }); + + it('should filter nodes that do not match a pinned searches', () => { + let nextState = fromJS({ + nodes: nodeSets.someNodes, + pinnedSearches: ['row'] + }); + nextState = fun(nextState); + expect(nextState.get('nodes').filter(node => node.get('filtered')).size).toEqual(1); + }); + }); + + describe('findNodeMatch', () => { + const fun = SearchUtils.findNodeMatch; + + it('does not add a non-matching field', () => { + let matches = fromJS({}); + matches = fun(matches, ['node1', 'field1'], + 'some value', 'some query', null, 'some label'); + expect(matches.size).toBe(0); + }); + + it('adds a matching field', () => { + let matches = fromJS({}); + matches = fun(matches, ['node1', 'field1'], + 'samevalue', 'samevalue', null, 'some label'); + expect(matches.size).toBe(1); + expect(matches.getIn(['node1', 'field1'])).toBeDefined(); + const {text, label, start, length} = matches.getIn(['node1', 'field1']); + expect(text).toBe('samevalue'); + expect(label).toBe('some label'); + expect(start).toBe(0); + expect(length).toBe(9); + }); + + it('does not add a field when the prefix does not match the label', () => { + let matches = fromJS({}); + matches = fun(matches, ['node1', 'field1'], + 'samevalue', 'samevalue', 'some prefix', 'some label'); + expect(matches.size).toBe(0); + }); + + it('adds a field when the prefix matches the label', () => { + let matches = fromJS({}); + matches = fun(matches, ['node1', 'field1'], + 'samevalue', 'samevalue', 'prefix', 'prefixed label'); + expect(matches.size).toBe(1); + }); + }); + + describe('findNodeMatchMetric', () => { + const fun = SearchUtils.findNodeMatchMetric; + + it('does not add a non-matching field', () => { + let matches = fromJS({}); + matches = fun(matches, ['node1', 'field1'], + 1, 'metric1', 'metric2', 'lt', 2); + expect(matches.size).toBe(0); + }); + + it('adds a matching field', () => { + let matches = fromJS({}); + matches = fun(matches, ['node1', 'field1'], + 1, 'metric1', 'metric1', 'lt', 2); + expect(matches.size).toBe(1); + expect(matches.getIn(['node1', 'field1'])).toBeDefined(); + const { metric } = matches.getIn(['node1', 'field1']); + expect(metric).toBeTruthy(); + + matches = fun(matches, ['node2', 'field1'], + 1, 'metric1', 'metric1', 'gt', 0); + expect(matches.size).toBe(2); + + matches = fun(matches, ['node3', 'field1'], + 1, 'metric1', 'metric1', 'eq', 1); + expect(matches.size).toBe(3); + + matches = fun(matches, ['node3', 'field1'], + 1, 'metric1', 'metric1', 'other', 1); + expect(matches.size).toBe(3); + }); + }); + + describe('makeRegExp', () => { + const fun = SearchUtils.makeRegExp; + + it('should make a regexp from any string', () => { + expect(fun().source).toEqual((new RegExp).source); + expect(fun('que').source).toEqual((new RegExp('que')).source); + // invalid string + expect(fun('que[').source).toEqual((new RegExp('que\\[')).source); + }); + }); + + describe('matchPrefix', () => { + const fun = SearchUtils.matchPrefix; + + it('returns true if the prefix matches the label', () => { + expect(fun('label', 'prefix')).toBeFalsy(); + expect(fun('memory', 'mem')).toBeTruthy(); + expect(fun('mem', 'memory')).toBeFalsy(); + expect(fun('com.domain.label', 'label')).toBeTruthy(); + expect(fun('com.domain.Label', 'domainlabel')).toBeTruthy(); + expect(fun('com-Domain-label', 'domainlabel')).toBeTruthy(); + expect(fun('memory', 'mem.ry')).toBeTruthy(); + }); + }); + + describe('parseQuery', () => { + const fun = SearchUtils.parseQuery; + + it('should parse a metric value from a string', () => { + expect(fun('')).toEqual(null); + expect(fun('text')).toEqual({query: 'text'}); + expect(fun('prefix:text')).toEqual({prefix: 'prefix', query: 'text'}); + expect(fun(':text')).toEqual(null); + expect(fun('text:')).toEqual(null); + expect(fun('cpu > 1')).toEqual({metric: 'cpu', value: 1, comp: 'gt'}); + expect(fun('cpu >')).toEqual(null); + }); + }); + + describe('parseValue', () => { + const fun = SearchUtils.parseValue; + + it('should parse a metric value from a string', () => { + expect(fun('1')).toEqual(1); + expect(fun('1.34%')).toEqual(1.34); + expect(fun('10kB')).toEqual(1024 * 10); + expect(fun('1K')).toEqual(1024); + expect(fun('2KB')).toEqual(2048); + expect(fun('1MB')).toEqual(Math.pow(1024, 2)); + expect(fun('1m')).toEqual(Math.pow(1024, 2)); + expect(fun('1GB')).toEqual(Math.pow(1024, 3)); + expect(fun('1TB')).toEqual(Math.pow(1024, 4)); + }); + }); + + describe('searchTopology', () => { + const fun = SearchUtils.searchTopology; + + it('should return no matches on an empty topology', () => { + const nodes = fromJS({}); + const matches = fun(nodes, {query: 'value'}); + expect(matches.size).toEqual(0); + }); + + it('should match on a node label', () => { + const nodes = nodeSets.someNodes; + let matches = fun(nodes, {query: 'node label 1'}); + expect(matches.size).toEqual(1); + matches = fun(nodes, {query: 'node label'}); + expect(matches.size).toEqual(2); + }); + + it('should match on a metadata field', () => { + const nodes = nodeSets.someNodes; + const matches = fun(nodes, {query: 'value'}); + expect(matches.size).toEqual(2); + expect(matches.getIn(['n1', 'metadata', 'fieldId1']).text).toEqual('value 1'); + }); + + it('should match on a metric field', () => { + const nodes = nodeSets.someNodes; + const matches = fun(nodes, {metric: 'metric1', value: 1, comp: 'eq'}); + expect(matches.size).toEqual(1); + expect(matches.getIn(['n1', 'metrics', 'metric1']).metric).toBeTruthy(); + }); + + it('should match on a tables field', () => { + const nodes = nodeSets.someNodes; + const matches = fun(nodes, {query: 'Row Value 1'}); + expect(matches.size).toEqual(1); + expect(matches.getIn(['n2', 'metadata', 'row1']).text).toBe('Row Value 1'); + }); + }); + + describe('updateNodeMatches', () => { + const fun = SearchUtils.updateNodeMatches; + + it('should return no matches on an empty topology', () => { + let nextState = fromJS({ + nodesByTopology: {}, + searchNodeMatches: {}, + searchQuery: '' + }); + nextState = fun(nextState); + expect(nextState.get('searchNodeMatches').size).toEqual(0); + }); + + it('should return no matches when no query is present', () => { + let nextState = fromJS({ + nodesByTopology: {topo1: nodeSets.someNodes}, + searchNodeMatches: {}, + searchQuery: '' + }); + nextState = fun(nextState); + expect(nextState.get('searchNodeMatches').size).toEqual(0); + }); + + it('should return no matches when query matches nothing', () => { + let nextState = fromJS({ + nodesByTopology: {topo1: nodeSets.someNodes}, + searchNodeMatches: {}, + searchQuery: 'cantmatch' + }); + nextState = fun(nextState); + expect(nextState.get('searchNodeMatches').size).toEqual(0); + }); + + it('should return a matches when a query matches something', () => { + let nextState = fromJS({ + nodesByTopology: {topo1: nodeSets.someNodes}, + searchNodeMatches: {}, + searchQuery: 'value 2' + }); + nextState = fun(nextState); + expect(nextState.get('searchNodeMatches').size).toEqual(1); + expect(nextState.get('searchNodeMatches').get('topo1').size).toEqual(1); + + // then clear up again + nextState = nextState.set('searchQuery', ''); + nextState = fun(nextState); + expect(nextState.get('searchNodeMatches').size).toEqual(0); + }); + }); +}); diff --git a/client/app/scripts/utils/router-utils.js b/client/app/scripts/utils/router-utils.js index ffe42c164..9e92a0bf9 100644 --- a/client/app/scripts/utils/router-utils.js +++ b/client/app/scripts/utils/router-utils.js @@ -8,13 +8,18 @@ import { route } from '../actions/app-actions'; // const SLASH = '/'; const SLASH_REPLACEMENT = ''; +const PERCENT = '%'; +const PERCENT_REPLACEMENT = ''; function encodeURL(url) { - return url.replace(new RegExp(SLASH, 'g'), SLASH_REPLACEMENT); + return url + .replace(new RegExp(PERCENT, 'g'), PERCENT_REPLACEMENT) + .replace(new RegExp(SLASH, 'g'), SLASH_REPLACEMENT); } function decodeURL(url) { - return decodeURIComponent(url.replace(new RegExp(SLASH_REPLACEMENT, 'g'), SLASH)); + return decodeURIComponent(url.replace(new RegExp(SLASH_REPLACEMENT, 'g'), SLASH)) + .replace(new RegExp(PERCENT_REPLACEMENT, 'g'), PERCENT); } function shouldReplaceState(prevState, nextState) { @@ -71,7 +76,7 @@ export function getRouter(dispatch, initialState) { }); page('/state/:state', (ctx) => { - const state = JSON.parse(ctx.params.state); + const state = JSON.parse(decodeURL(ctx.params.state)); dispatch(route(state)); }); diff --git a/client/app/scripts/utils/search-utils.js b/client/app/scripts/utils/search-utils.js index 4ec786dd3..92bdc8c9b 100644 --- a/client/app/scripts/utils/search-utils.js +++ b/client/app/scripts/utils/search-utils.js @@ -18,6 +18,10 @@ const COMPARISONS_REGEX = new RegExp(`[${COMPARISONS.keySeq().toJS().join('')}]` const PREFIX_DELIMITER = ':'; +/** + * Returns a RegExp from a given string. If the string is not a valid regexp, + * it is escaped. Returned regexp is case-insensitive. + */ function makeRegExp(expression, options = 'i') { try { return new RegExp(expression, options); @@ -26,20 +30,27 @@ function makeRegExp(expression, options = 'i') { } } +/** + * Returns the float of a metric value string, e.g. 2 KB -> 2048 + */ function parseValue(value) { let parsed = parseFloat(value); - if (_.endsWith(value, 'KB')) { + if ((/k/i).test(value)) { parsed *= 1024; - } else if (_.endsWith(value, 'MB')) { + } else if ((/m/i).test(value)) { parsed *= 1024 * 1024; - } else if (_.endsWith(value, 'GB')) { + } else if ((/g/i).test(value)) { parsed *= 1024 * 1024 * 1024; - } else if (_.endsWith(value, 'TB')) { + } else if ((/t/i).test(value)) { parsed *= 1024 * 1024 * 1024 * 1024; } return parsed; } +/** + * True if a prefix matches a field label + * Slugifies the label (removes all non-alphanumerical chars). + */ function matchPrefix(label, prefix) { if (label && prefix) { return (makeRegExp(prefix)).test(slugify(label)); @@ -47,6 +58,12 @@ function matchPrefix(label, prefix) { return false; } +/** + * Adds a match to nodeMatches under the keyPath. The text is matched against + * the query. If a prefix is given, it is matched against the label (skip on + * no match). + * Returns a new instance of nodeMatches. + */ function findNodeMatch(nodeMatches, keyPath, text, query, prefix, label) { if (!prefix || matchPrefix(label, prefix)) { const queryRe = makeRegExp(query); @@ -55,7 +72,7 @@ function findNodeMatch(nodeMatches, keyPath, text, query, prefix, label) { const firstMatch = matches[0]; const index = text.search(queryRe); nodeMatches = nodeMatches.setIn(keyPath, - {text, label, matches: [{start: index, length: firstMatch.length}]}); + {text, label, start: index, length: firstMatch.length}); } } return nodeMatches; @@ -99,6 +116,7 @@ function findNodeMatchMetric(nodeMatches, keyPath, fieldValue, fieldLabel, metri return nodeMatches; } + export function searchTopology(nodes, { prefix, query, metric, comp, value }) { let nodeMatches = makeMap(); nodes.forEach((node, nodeId) => { @@ -106,8 +124,10 @@ export function searchTopology(nodes, { prefix, query, metric, comp, value }) { // top level fields SEARCH_FIELDS.forEach((field, label) => { const keyPath = [nodeId, label]; - nodeMatches = findNodeMatch(nodeMatches, keyPath, node.get(field), - query, prefix, label); + if (node.has(field)) { + nodeMatches = findNodeMatch(nodeMatches, keyPath, node.get(field), + query, prefix, label); + } }); // metadata @@ -146,6 +166,12 @@ export function searchTopology(nodes, { prefix, query, metric, comp, value }) { return nodeMatches; } +/** + * Returns an object with fields depending on the query: + * parseQuery('text') -> {query: 'text'} + * parseQuery('p:text') -> {query: 'text', prefix: 'p'} + * parseQuery('cpu > 1') -> {metric: 'cpu', value: '1', comp: 'gt'} + */ export function parseQuery(query) { if (query) { const prefixQuery = query.split(PREFIX_DELIMITER); @@ -195,14 +221,17 @@ export function parseQuery(query) { export function updateNodeMatches(state) { const parsed = parseQuery(state.get('searchQuery')); if (parsed) { - state.get('topologyUrlsById').forEach((url, topologyId) => { - const topologyNodes = state.getIn(['nodesByTopology', topologyId]); - if (topologyNodes) { - const nodeMatches = searchTopology(topologyNodes, parsed); - state = state.setIn(['searchNodeMatches', topologyId], nodeMatches); - } - }); - } else { + if (state.has('nodesByTopology')) { + state.get('nodesByTopology').forEach((nodes, topologyId) => { + const nodeMatches = searchTopology(nodes, parsed); + if (nodeMatches.size > 0) { + state = state.setIn(['searchNodeMatches', topologyId], nodeMatches); + } else { + state = state.deleteIn(['searchNodeMatches', topologyId]); + } + }); + } + } else if (state.has('searchNodeMatches')) { state = state.update('searchNodeMatches', snm => snm.clear()); } @@ -235,3 +264,15 @@ export function applyPinnedSearches(state) { return state; } + +export const testable = { + applyPinnedSearches, + findNodeMatch, + findNodeMatchMetric, + matchPrefix, + makeRegExp, + parseQuery, + parseValue, + searchTopology, + updateNodeMatches +}; diff --git a/client/app/styles/contrast.less b/client/app/styles/contrast.less index b99284b2b..2e3575acd 100644 --- a/client/app/styles/contrast.less +++ b/client/app/styles/contrast.less @@ -28,3 +28,6 @@ @btn-opacity-selected: 1; @link-opacity-default: 1; + +@search-border-color: @background-darker-color; +@search-border-width: 2px; diff --git a/client/app/styles/main.less b/client/app/styles/main.less index b6ce58573..519c0bb67 100644 --- a/client/app/styles/main.less +++ b/client/app/styles/main.less @@ -60,6 +60,9 @@ @link-opacity-default: 0.8; +@search-border-color: transparent; +@search-border-width: 1px; + /* add this class to truncate text with ellipsis, container needs width */ .truncate { white-space: nowrap; @@ -319,12 +322,12 @@ h2 { top: 0px; } - .logo { + .logo, .node-label-svg { display: none; } svg.exported { - .logo { + .logo, .node-label-svg { display: inline; } } @@ -332,29 +335,42 @@ h2 { text { font-family: @base-font; fill: @text-secondary-color; - - &.node-label { - fill: @text-color; - } - - &.node-sublabel { - fill: @text-secondary-color; - } } .nodes-chart-nodes > .node { - cursor: pointer; transition: opacity .5s @base-ease; + text-align: center; - .hover-box { - fill-opacity: 0; + .node-label, + .node-sublabel { + line-height: 125%; } - &.hovered .node-label, &.hovered .node-sublabel { - stroke: @background-average-color; - stroke-width: 8px; - stroke-opacity: 0.7; - paint-order: stroke; + .node-label { + color: @text-color; + font-size: 14px; + } + + .node-label-wrapper { + display: inline-block; + cursor: pointer; + padding-top: 6px; + } + + .node-sublabel { + color: @text-secondary-color; + font-size: 12px; + } + + &.hovered { + .node-label, .node-sublabel { + span:not(.match) { + background-color: fade(@background-average-color, 70%); + } + } + .matched-results { + background-color: fade(@background-average-color, 70%); + } } &.pseudo { @@ -443,6 +459,7 @@ h2 { .shape { transform: scale(1); + cursor: pointer; /* cloud paths have stroke-width set dynamically */ &:not(.shape-cloud) .border { @@ -516,8 +533,9 @@ h2 { &-more { text-transform: uppercase; - font-size: 0.7rem; - color: @text-tertiary-color; + font-size: 0.6rem; + color: darken(@weave-blue, 10%); + margin-top: -2px; } } @@ -1229,13 +1247,10 @@ h2 { display: flex; border-radius: @border-radius; width: 100%; - border: 1px solid transparent; + border: @search-border-width solid @search-border-color; padding: 2px 4px; text-align: left; - - &-items { - padding: 2px 4px; - } + flex-wrap: wrap; &-field { font-size: 0.8rem; @@ -1247,6 +1262,7 @@ h2 { background: transparent; color: @text-color; flex: 1; + width: 60px; &:focus { outline: none; @@ -1259,14 +1275,15 @@ h2 { text-align: center; color: @text-secondary-color; position: relative; - top: 4px; + top: 2px; left: 4px; + padding: 2px; } &-label { user-select: none; display: inline-block; - padding: 2px 1em; + padding: 2px 0.75em; font-size: 0.8rem; position: absolute; text-align: left; @@ -1308,7 +1325,12 @@ h2 { .search-item { background-color: fade(@weave-blue, 20%); border-radius: @border-radius / 2; - margin-left: 4px; + margin: 1px 0 1px 8px; + display: inline-block; + + & + .search-item { + margin-left: 4px; + } &-label { padding: 2px 4px;