From 8ba82b20ec78e063f71aedf005e945d8852ec6ea Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Tue, 28 Apr 2020 13:05:23 +0100 Subject: [PATCH 01/13] Refactor node-list scss into multiple files --- src/components/node-list/index.js | 2 +- src/components/node-list/node-list.scss | 338 ------------------ src/components/node-list/styles/_group.scss | 88 +++++ src/components/node-list/styles/_row.scss | 134 +++++++ src/components/node-list/styles/_search.scss | 16 + src/components/node-list/styles/_toggle.scss | 70 ++++ .../node-list/styles/_variables.scss | 10 + .../node-list/styles/node-list.scss | 20 ++ 8 files changed, 339 insertions(+), 339 deletions(-) delete mode 100644 src/components/node-list/node-list.scss create mode 100644 src/components/node-list/styles/_group.scss create mode 100644 src/components/node-list/styles/_row.scss create mode 100644 src/components/node-list/styles/_search.scss create mode 100644 src/components/node-list/styles/_toggle.scss create mode 100644 src/components/node-list/styles/_variables.scss create mode 100644 src/components/node-list/styles/node-list.scss diff --git a/src/components/node-list/index.js b/src/components/node-list/index.js index 7a118e0f52..49c8fba989 100644 --- a/src/components/node-list/index.js +++ b/src/components/node-list/index.js @@ -6,7 +6,7 @@ import { Scrollbars } from 'react-custom-scrollbars'; import { getGroupedNodes } from '../../selectors/nodes'; import NodeListToggleAll from './node-list-toggle'; import NodeListGroups from './node-list-groups'; -import './node-list.css'; +import './styles/node-list.css'; const { escapeRegExp, getHighlightedText, handleKeyEvent } = utils; diff --git a/src/components/node-list/node-list.scss b/src/components/node-list/node-list.scss deleted file mode 100644 index d4c3b7752f..0000000000 --- a/src/components/node-list/node-list.scss +++ /dev/null @@ -1,338 +0,0 @@ -@import '../../styles/extends'; -@import '../../styles/mixins'; -@import '../../styles/variables'; - -$toggle-size: 2.4em; -$toggle-margin-right: 0.8em; -$row-padding-y: 0.8em; -$row-padding-x: 1.2em; -$row-icon-margin: 0.5em; -$row-icon-size: 2.4em; -$visibility-icon-padding: 0.3em; -$visibility-icon-size: 2.4em + $visibility-icon-padding * 2; -$row-offset-left: $row-padding-x + $toggle-size + $toggle-margin-right; -$row-offset-right: $row-padding-x + $row-icon-size + $row-icon-margin; - -.pipeline-nodelist-search { - padding: 14px 12px 12px; - - .kui-theme--light & { - background: $color-bg-light-alt2; - } - - .kui-theme--dark & { - background: $color-bg-dark-alt2; - } - - .kui-searchbar, - .kui-input-wrapper { - max-width: none; - } -} - -.pipeline-nodelist-scrollbars { - min-height: 400px; - - .kui-theme--light & { - background: $color-bg-light-alt2; - } - - .kui-theme--dark & { - background: $color-bg-dark-alt2; - } -} - -.pipeline-nodelist__toggle { - margin: 0 24px; -} - -.pipeline-nodelist__toggle__title { - margin: 0 0 12px; - font-weight: 600; - font-size: 1.8em; -} - -.pipeline-nodelist__toggle__row { - display: flex; - flex-direction: row; - justify-content: space-between; - margin: 12px 0; - padding: 10px 0; - - .kui-theme--light & { - border-top: 1px solid $color-line-light; - border-bottom: 1px solid $color-line-light; - } - - .kui-theme--dark & { - border-top: 1px solid $color-line-dark; - border-bottom: 1px solid $color-line-dark; - } -} - -.pipeline-nodelist__toggle__button { - margin: -1px -6px -2px; - padding: 1px 6px 2px; - color: inherit; - font-size: 1.6em; - font-family: inherit; - background: none; - border: none; - box-shadow: none; - cursor: pointer; - opacity: 0.8; - - &:focus { - outline: 4px solid $color-link; - opacity: 1; - - [data-whatintent='mouse'] & { - outline: none; - } - } - - &:hover { - opacity: 1; - } -} - -.pipeline-nodelist__toggle__icon { - margin: 0 4px -6px -6px; - fill: white; - - .kui-theme--light & { - fill: rgba(black, 0.9); - } - - .kui-theme--dark & { - fill: white; - } - - &--uncheck { - transform: scale(0.75); - } -} - -%nolist { - margin: 0; - padding: 0; - list-style: none; -} - -.pipeline-nodelist { - @extend %nolist; - - li { - @extend %nolist; - } - - .kui-theme--light & { - background: $color-bg-light-alt2; - } - - .kui-theme--dark & { - background: $color-bg-dark-alt2; - } - - &--nested { - margin: 0 0 1.2em; - } - - &--fade-in { - animation: fadeIn 0.4s forwards; - } - - &--fade-out { - animation: fadeIn 0.4s backwards reverse; - } -} - -@keyframes fadeIn { - 0% { - opacity: 0; - } - - 100% { - opacity: 1; - } -} - -.pipeline-nodelist__heading { - margin: 0; -} - -.pipeline-nodelist__row { - position: relative; - display: flex; - align-items: center; - - &:hover { - background: rgba(#fff, 0.05); - } - - &--active, - &--active:hover { - background: rgba(#fff, 0.1); - } - - &--disabled { - pointer-events: none; - } -} - -.pipeline-type-group-toggle { - position: absolute; - left: $row-padding-x; - width: $toggle-size; - height: $toggle-size; - margin-right: $toggle-margin-right; - padding: 0; - color: inherit; - font-family: inherit; - line-height: 1em; - text-align: center; - background: none; - border: none; - border-radius: 50%; - box-shadow: none; - cursor: pointer; - transition: transform ease 0.3s; - - &:focus { - outline: none; - - [data-whatintent='keyboard'] & { - box-shadow: 0 0 0 3px $color-link inset; - } - } - - &:before { - font-size: 1.5em; - content: '▾'; - } - - &:hover { - background: rgba(#fff, 0.1); - box-shadow: 0 0 0 1px rgba(#fff, 0.03) inset; - } - - &--alt { - transform: rotate(-180deg); - } -} - -.pipeline-nodelist__row__icon { - display: block; - flex-shrink: 0; - width: $row-icon-size; - height: $row-icon-size; - transition: opacity ease 0.1s; - - .kui-theme--light & { - fill: $color-text-light; - } - - .kui-theme--dark & { - fill: $color-text-dark; - } -} - -.pipeline-nodelist__row__type-icon { - margin-right: $row-icon-margin; - - &--nested { - opacity: 0; - } - - .pipeline-nodelist__row:hover &, - [data-whatintent='keyboard'] .pipeline-nodelist__row__text:focus & { - opacity: 1; - - &--unchecked { - opacity: 0.55; - } - } -} - -.pipeline-nodelist__row__text { - display: flex; - align-items: center; - width: 100%; - padding: $row-padding-y $row-offset-right $row-padding-y $row-offset-left; - color: inherit; - font-size: inherit; - text-align: inherit; - background: none; - border: none; - border-radius: 0; - box-shadow: none; - transition: opacity ease 0.1s; - - &:focus { - outline: 4px solid $color-link; - - [data-whatintent='mouse'] & { - outline: none; - } - } -} - -.pipeline-nodelist__row__checkbox { - @include screenReaderOnly; -} - -.pipeline-nodelist__row__label { - overflow: hidden; - font-size: 1.6em; - white-space: nowrap; - text-overflow: ellipsis; - - &--disabled, - &--unchecked { - opacity: 0.55; - } - - b { - font-weight: normal; - opacity: 0.3; - } -} - -.pipeline-nodelist__row__visibility { - position: absolute; - right: $row-padding-x; - cursor: pointer; -} - -.pipeline-nodelist__row__visibility-icon { - width: $visibility-icon-size; - height: $visibility-icon-size; - padding: $visibility-icon-padding; - border-radius: 50%; - opacity: 0; - transition: opacity ease 0.2s; - - &--unchecked, - .pipeline-nodelist__row__text:focus + label &, - .pipeline-nodelist__row:hover & { - opacity: 0.55; - } - - .pipeline-nodelist__row:hover &:hover { - opacity: 1; - } - - .pipeline-nodelist__row__checkbox:focus + & { - outline: none; - transition: box-shadow ease 0.2s; - - [data-whatintent='keyboard'] & { - box-shadow: 0 0 0 3px $color-link inset; - opacity: 1; - } - } -} - -.pipeline-nodelist__row__checkbox { - @include screenReaderOnly; -} diff --git a/src/components/node-list/styles/_group.scss b/src/components/node-list/styles/_group.scss new file mode 100644 index 0000000000..c7fa0fca2a --- /dev/null +++ b/src/components/node-list/styles/_group.scss @@ -0,0 +1,88 @@ +%nolist { + margin: 0; + padding: 0; + list-style: none; +} + +.pipeline-nodelist { + @extend %nolist; + + li { + @extend %nolist; + } + + .kui-theme--light & { + background: $color-bg-light-alt2; + } + + .kui-theme--dark & { + background: $color-bg-dark-alt2; + } + + &--nested { + margin: 0 0 1.2em; + } + + &--fade-in { + animation: fadeIn 0.4s forwards; + } + + &--fade-out { + animation: fadeIn 0.4s backwards reverse; + } +} + +@keyframes fadeIn { + 0% { + opacity: 0; + } + + 100% { + opacity: 1; + } +} + +.pipeline-nodelist__heading { + margin: 0; +} + +.pipeline-type-group-toggle { + position: absolute; + left: $row-padding-x; + width: $toggle-size; + height: $toggle-size; + margin-right: $toggle-margin-right; + padding: 0; + color: inherit; + font-family: inherit; + line-height: 1em; + text-align: center; + background: none; + border: none; + border-radius: 50%; + box-shadow: none; + cursor: pointer; + transition: transform ease 0.3s; + + &:focus { + outline: none; + + [data-whatintent='keyboard'] & { + box-shadow: 0 0 0 3px $color-link inset; + } + } + + &:before { + font-size: 1.5em; + content: '▾'; + } + + &:hover { + background: rgba(#fff, 0.1); + box-shadow: 0 0 0 1px rgba(#fff, 0.03) inset; + } + + &--alt { + transform: rotate(-180deg); + } +} diff --git a/src/components/node-list/styles/_row.scss b/src/components/node-list/styles/_row.scss new file mode 100644 index 0000000000..28050f579c --- /dev/null +++ b/src/components/node-list/styles/_row.scss @@ -0,0 +1,134 @@ +.pipeline-nodelist__row { + position: relative; + display: flex; + align-items: center; + + &:hover { + background: rgba(#fff, 0.05); + } + + &--active, + &--active:hover { + background: rgba(#fff, 0.1); + } + + &--disabled { + pointer-events: none; + } +} + +.pipeline-nodelist__row__icon { + display: block; + flex-shrink: 0; + width: $row-icon-size; + height: $row-icon-size; + transition: opacity ease 0.1s; + + .kui-theme--light & { + fill: $color-text-light; + } + + .kui-theme--dark & { + fill: $color-text-dark; + } +} + +.pipeline-nodelist__row__type-icon { + margin-right: $row-icon-margin; + + &--nested { + opacity: 0; + } + + .pipeline-nodelist__row:hover &, + [data-whatintent='keyboard'] .pipeline-nodelist__row__text:focus & { + opacity: 1; + + &--unchecked { + opacity: 0.55; + } + } +} + +.pipeline-nodelist__row__text { + display: flex; + align-items: center; + width: 100%; + padding: $row-padding-y $row-offset-right $row-padding-y $row-offset-left; + color: inherit; + font-size: inherit; + text-align: inherit; + background: none; + border: none; + border-radius: 0; + box-shadow: none; + transition: opacity ease 0.1s; + + &:focus { + outline: 4px solid $color-link; + + [data-whatintent='mouse'] & { + outline: none; + } + } +} + +.pipeline-nodelist__row__checkbox { + @include screenReaderOnly; +} + +.pipeline-nodelist__row__label { + overflow: hidden; + font-size: 1.6em; + white-space: nowrap; + text-overflow: ellipsis; + + &--disabled, + &--unchecked { + opacity: 0.55; + } + + b { + font-weight: normal; + opacity: 0.3; + } +} + +.pipeline-nodelist__row__visibility { + position: absolute; + right: $row-padding-x; + cursor: pointer; +} + +.pipeline-nodelist__row__visibility-icon { + width: $visibility-icon-size; + height: $visibility-icon-size; + padding: $visibility-icon-padding; + border-radius: 50%; + opacity: 0; + transition: opacity ease 0.2s; + + &--unchecked, + .pipeline-nodelist__row__text:focus + label &, + .pipeline-nodelist__row:hover & { + opacity: 0.55; + } + + .pipeline-nodelist__row:hover &:hover { + opacity: 1; + } + + .pipeline-nodelist__row__checkbox:focus + & { + outline: none; + transition: box-shadow ease 0.2s; + + [data-whatintent='keyboard'] & { + box-shadow: 0 0 0 3px $color-link inset; + opacity: 1; + } + } +} + +.pipeline-nodelist__row__checkbox { + @include screenReaderOnly; +} diff --git a/src/components/node-list/styles/_search.scss b/src/components/node-list/styles/_search.scss new file mode 100644 index 0000000000..893e84417c --- /dev/null +++ b/src/components/node-list/styles/_search.scss @@ -0,0 +1,16 @@ +.pipeline-nodelist-search { + padding: 14px 12px 12px; + + .kui-theme--light & { + background: $color-bg-light-alt2; + } + + .kui-theme--dark & { + background: $color-bg-dark-alt2; + } + + .kui-searchbar, + .kui-input-wrapper { + max-width: none; + } +} diff --git a/src/components/node-list/styles/_toggle.scss b/src/components/node-list/styles/_toggle.scss new file mode 100644 index 0000000000..5541969756 --- /dev/null +++ b/src/components/node-list/styles/_toggle.scss @@ -0,0 +1,70 @@ +.pipeline-nodelist__toggle { + margin: 0 24px; +} + +.pipeline-nodelist__toggle__title { + margin: 0 0 12px; + font-weight: 600; + font-size: 1.8em; +} + +.pipeline-nodelist__toggle__row { + display: flex; + flex-direction: row; + justify-content: space-between; + margin: 12px 0; + padding: 10px 0; + + .kui-theme--light & { + border-top: 1px solid $color-line-light; + border-bottom: 1px solid $color-line-light; + } + + .kui-theme--dark & { + border-top: 1px solid $color-line-dark; + border-bottom: 1px solid $color-line-dark; + } +} + +.pipeline-nodelist__toggle__button { + margin: -1px -6px -2px; + padding: 1px 6px 2px; + color: inherit; + font-size: 1.6em; + font-family: inherit; + background: none; + border: none; + box-shadow: none; + cursor: pointer; + opacity: 0.8; + + &:focus { + outline: 4px solid $color-link; + opacity: 1; + + [data-whatintent='mouse'] & { + outline: none; + } + } + + &:hover { + opacity: 1; + } +} + +.pipeline-nodelist__toggle__icon { + margin: 0 4px -6px -6px; + fill: white; + + .kui-theme--light & { + fill: rgba(black, 0.9); + } + + .kui-theme--dark & { + fill: white; + } + + &--uncheck { + transform: scale(0.75); + } +} diff --git a/src/components/node-list/styles/_variables.scss b/src/components/node-list/styles/_variables.scss new file mode 100644 index 0000000000..cfb4964e66 --- /dev/null +++ b/src/components/node-list/styles/_variables.scss @@ -0,0 +1,10 @@ +$toggle-size: 2.4em; +$toggle-margin-right: 0.8em; +$row-padding-y: 0.8em; +$row-padding-x: 1.2em; +$row-icon-margin: 0.5em; +$row-icon-size: 2.4em; +$visibility-icon-padding: 0.3em; +$visibility-icon-size: 2.4em + $visibility-icon-padding * 2; +$row-offset-left: $row-padding-x + $toggle-size + $toggle-margin-right; +$row-offset-right: $row-padding-x + $row-icon-size + $row-icon-margin; diff --git a/src/components/node-list/styles/node-list.scss b/src/components/node-list/styles/node-list.scss new file mode 100644 index 0000000000..757403054b --- /dev/null +++ b/src/components/node-list/styles/node-list.scss @@ -0,0 +1,20 @@ +@import '../../../styles/extends'; +@import '../../../styles/mixins'; +@import '../../../styles/variables'; +@import './variables'; +@import './search'; +@import './toggle'; +@import './group'; +@import './row'; + +.pipeline-nodelist-scrollbars { + min-height: 400px; + + .kui-theme--light & { + background: $color-bg-light-alt2; + } + + .kui-theme--dark & { + background: $color-bg-dark-alt2; + } +} From 9e581cd8de0b997039c55ff91edfa3031373b554 Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Tue, 28 Apr 2020 13:59:09 +0100 Subject: [PATCH 02/13] Refactor node-list search into separate files --- src/components/node-list/filter-nodes.js | 80 ++++++++++ src/components/node-list/index.js | 153 +++---------------- src/components/node-list/node-list-search.js | 39 +++++ 3 files changed, 144 insertions(+), 128 deletions(-) create mode 100644 src/components/node-list/filter-nodes.js create mode 100644 src/components/node-list/node-list-search.js diff --git a/src/components/node-list/filter-nodes.js b/src/components/node-list/filter-nodes.js new file mode 100644 index 0000000000..b68285921a --- /dev/null +++ b/src/components/node-list/filter-nodes.js @@ -0,0 +1,80 @@ +import utils from '@quantumblack/kedro-ui/lib/utils'; +const { escapeRegExp, getHighlightedText } = utils; + +/** + * Get a list of IDs of the visible nodes + * @param {object} nodes Grouped nodes + * @return {array} List of node IDs + */ +export const getNodeIDs = nodes => { + const getNodeIDs = type => nodes[type].map(node => node.id); + const concatNodeIDs = (nodeIDs, type) => nodeIDs.concat(getNodeIDs(type)); + + return Object.keys(nodes).reduce(concatNodeIDs, []); +}; + +/** + * Add a new highlightedLabel field to each of the node objects + * @param {object} nodes Grouped lists of nodes + * @param {string} searchValue Search term + * @return {object} The grouped nodes with highlightedLabel fields added + */ +export const highlightMatch = (nodes, searchValue) => { + const addHighlightedLabel = node => ({ + highlightedLabel: getHighlightedText(node.name, searchValue), + ...node + }); + const addLabelsToNodes = (newNodes, type) => ({ + ...newNodes, + [type]: nodes[type].map(addHighlightedLabel) + }); + + return Object.keys(nodes).reduce(addLabelsToNodes, {}); +}; + +/** + * Check whether a name matches the search text + * @param {string} name + * @param {string} searchValue + * @return {boolean} True if match + */ +export const nodeMatchesSearch = (node, searchValue) => { + const valueRegex = searchValue + ? new RegExp(escapeRegExp(searchValue), 'gi') + : ''; + return Boolean(node.name.match(valueRegex)); +}; + +/** + * Return only the results that match the search text + * @param {object} nodes Grouped lists of nodes + * @param {string} searchValue Search term + * @return {object} Grouped nodes + */ +export const filterNodes = (nodes, searchValue) => { + const filterNodesByType = type => + nodes[type].filter(node => nodeMatchesSearch(node, searchValue)); + const filterNodeLists = (newNodes, type) => ({ + ...newNodes, + [type]: filterNodesByType(type) + }); + + return Object.keys(nodes).reduce(filterNodeLists, {}); +}; + +/** + * Return highlighted/formatted nodes, and filtered node IDs + * @param {object} nodes Grouped lists of nodes + * @param {string} searchValue Search term + * @return {object} Grouped nodes, and node IDs + */ +const getFormattedNodes = (nodes, searchValue) => { + const filteredNodes = filterNodes(nodes, searchValue); + + return { + formattedNodes: highlightMatch(filteredNodes, searchValue), + nodeIDs: getNodeIDs(filteredNodes) + }; +}; + +export default getFormattedNodes; diff --git a/src/components/node-list/index.js b/src/components/node-list/index.js index 49c8fba989..543ae034ee 100644 --- a/src/components/node-list/index.js +++ b/src/components/node-list/index.js @@ -1,143 +1,40 @@ -import React from 'react'; +import React, { useState } from 'react'; import { connect } from 'react-redux'; -import SearchBar from '@quantumblack/kedro-ui/lib/components/search-bar'; -import utils from '@quantumblack/kedro-ui/lib/utils'; import { Scrollbars } from 'react-custom-scrollbars'; import { getGroupedNodes } from '../../selectors/nodes'; +import getFormattedNodes from './filter-nodes'; +import NodeListSearch from './node-list-search'; import NodeListToggleAll from './node-list-toggle'; import NodeListGroups from './node-list-groups'; import './styles/node-list.css'; -const { escapeRegExp, getHighlightedText, handleKeyEvent } = utils; - -/** - * Get a list of IDs of the visible nodes - * @param {object} nodes Grouped nodes - * @return {array} List of node IDs - */ -export const getNodeIDs = nodes => { - const getNodeIDs = type => nodes[type].map(node => node.id); - const concatNodeIDs = (nodeIDs, type) => nodeIDs.concat(getNodeIDs(type)); - - return Object.keys(nodes).reduce(concatNodeIDs, []); -}; - /** * Scrollable list of toggleable nodes, with search & filter functionality */ -class NodeList extends React.Component { - constructor(props) { - super(props); - - this.state = { - searchValue: '' - }; - - this.handleKeyDown = this.handleKeyDown.bind(this); - this.updateSearchValue = this.updateSearchValue.bind(this); - } - - /** - * Add a new highlightedLabel field to each of the node objects - * @param {object} nodes Grouped lists of nodes - * @return {object} The grouped nodes with highlightedLabel fields added - */ - highlightMatch(nodes) { - const { searchValue } = this.state; - const addHighlightedLabel = node => ({ - highlightedLabel: getHighlightedText(node.name, searchValue), - ...node - }); - const addLabelsToNodes = (newNodes, type) => ({ - ...newNodes, - [type]: nodes[type].map(addHighlightedLabel) - }); - - return Object.keys(nodes).reduce(addLabelsToNodes, {}); - } - - /** - * Check whether a name matches the search text - * @param {string} name - * @param {string} searchValue - * @return {boolean} True if match - */ - nodeMatchesSearch(node, searchValue) { - const valueRegex = searchValue - ? new RegExp(escapeRegExp(searchValue), 'gi') - : ''; - return Boolean(node.name.match(valueRegex)); - } - - /** - * Return only the results that match the search text - * @return {object} Grouped nodes - */ - filterNodes() { - const { nodes } = this.props; - const { searchValue } = this.state; - const filterNodeLists = (newNodes, type) => ({ - ...newNodes, - [type]: filterNodesByType(type) - }); - const filterNodesByType = type => - nodes[type].filter(node => this.nodeMatchesSearch(node, searchValue)); - - return Object.keys(nodes).reduce(filterNodeLists, {}); - } - - /** - * Listen for keyboard events, and trigger relevant actions - * @param {number} keyCode The key event keycode - */ - handleKeyDown(event) { - handleKeyEvent(event.keyCode, { - escape: this.updateSearchValue.bind(this, '') - }); - } - - /** - * Apply the new search filter text to the component state - * @param {string} searchValue The term being searched - */ - updateSearchValue(searchValue) { - this.setState({ - searchValue - }); - } - - render() { - const filteredNodes = this.filterNodes(); - const formattedNodes = this.highlightMatch(filteredNodes); - const nodeIDs = getNodeIDs(filteredNodes); - - return ( - -
- -
- - - - - - ); - } -} +const NodeList = ({ nodes }) => { + const [searchValue, updateSearchValue] = useState(''); + const { formattedNodes, nodeIDs } = getFormattedNodes(nodes, searchValue); + + return ( + + + + + + + + ); +}; export const mapStateToProps = state => ({ - nodes: getGroupedNodes(state), - theme: state.theme + nodes: getGroupedNodes(state) }); export default connect(mapStateToProps)(NodeList); diff --git a/src/components/node-list/node-list-search.js b/src/components/node-list/node-list-search.js new file mode 100644 index 0000000000..bcba5887f5 --- /dev/null +++ b/src/components/node-list/node-list-search.js @@ -0,0 +1,39 @@ +import React from 'react'; +import SearchBar from '@quantumblack/kedro-ui/lib/components/search-bar'; +import utils from '@quantumblack/kedro-ui/lib/utils'; +import { connect } from 'react-redux'; +const { handleKeyEvent } = utils; + +/** + * Handle Node List Search + * @param {function} onUpdateSearchValue Event handler + * @param {string} searchValue Search text + * @param {string} theme Light/dark theme for Kedro-UI component + */ +const NodeListSearch = ({ onUpdateSearchValue, searchValue, theme }) => { + /** + * Listen for keyboard events, and trigger relevant actions + * @param {number} keyCode The key event keycode + */ + const handleKeyDown = event => { + handleKeyEvent(event.keyCode, { + escape: onUpdateSearchValue.bind(this, '') + }); + }; + + return ( +
+ +
+ ); +}; + +export const mapStateToProps = state => ({ + theme: state.theme +}); + +export default connect(mapStateToProps)(NodeListSearch); From cb69904ea9a510b50c777e4fbe1909e64934cc94 Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Tue, 28 Apr 2020 14:34:58 +0100 Subject: [PATCH 03/13] Fix tests --- src/components/node-list/filter-nodes.test.js | 25 +++++++++++++++++++ src/components/node-list/node-list.test.js | 25 +------------------ 2 files changed, 26 insertions(+), 24 deletions(-) create mode 100644 src/components/node-list/filter-nodes.test.js diff --git a/src/components/node-list/filter-nodes.test.js b/src/components/node-list/filter-nodes.test.js new file mode 100644 index 0000000000..1bb6ff4dbd --- /dev/null +++ b/src/components/node-list/filter-nodes.test.js @@ -0,0 +1,25 @@ +import { getNodeIDs } from './filter-nodes'; + +describe('filter-nodes', () => { + describe('getNodeIDs', () => { + const generateNodes = (type, count) => + Array.from(new Array(count)).map((d, i) => ({ + id: type + i + })); + + const nodes = { + data: generateNodes('data', 10), + task: generateNodes('task', 10), + parameters: generateNodes('parameters', 10) + }; + + it('returns a list of node IDs', () => { + const nodeIDs = getNodeIDs(nodes); + expect(nodeIDs).toHaveLength(30); + expect(nodeIDs).toEqual(expect.arrayContaining([expect.any(String)])); + expect(nodeIDs).toContain('data0'); + expect(nodeIDs).toContain('task1'); + expect(nodeIDs).toContain('parameters1'); + }); + }); +}); diff --git a/src/components/node-list/node-list.test.js b/src/components/node-list/node-list.test.js index 88452e495f..05f2009dea 100644 --- a/src/components/node-list/node-list.test.js +++ b/src/components/node-list/node-list.test.js @@ -1,31 +1,9 @@ import React from 'react'; -import NodeList, { getNodeIDs, mapStateToProps } from './index'; +import NodeList, { mapStateToProps } from './index'; import { mockState, setup } from '../../utils/state.mock'; import { getNodeData } from '../../selectors/nodes'; describe('NodeList', () => { - describe('getNodeIDs', () => { - const generateNodes = (type, count) => - Array.from(new Array(count)).map((d, i) => ({ - id: type + i - })); - - const nodes = { - data: generateNodes('data', 10), - task: generateNodes('task', 10), - parameters: generateNodes('parameters', 10) - }; - - it('returns a list of node IDs', () => { - const nodeIDs = getNodeIDs(nodes); - expect(nodeIDs).toHaveLength(30); - expect(nodeIDs).toEqual(expect.arrayContaining([expect.any(String)])); - expect(nodeIDs).toContain('data0'); - expect(nodeIDs).toContain('task1'); - expect(nodeIDs).toContain('parameters1'); - }); - }); - it('renders without crashing', () => { const wrapper = setup.mount(); const search = wrapper.find('.pipeline-nodelist-search'); @@ -243,7 +221,6 @@ describe('NodeList', () => { }) ]); const expectedResult = { - theme: expect.stringMatching(/light|dark/), nodes: expect.objectContaining({ data: nodeList, task: nodeList From 8f1efc04786d3d66e88451a9f55c1cc092b12c76 Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Tue, 28 Apr 2020 15:13:57 +0100 Subject: [PATCH 04/13] Add tests for getFormattedNodes --- src/components/node-list/filter-nodes.test.js | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/components/node-list/filter-nodes.test.js b/src/components/node-list/filter-nodes.test.js index 1bb6ff4dbd..7f14b67135 100644 --- a/src/components/node-list/filter-nodes.test.js +++ b/src/components/node-list/filter-nodes.test.js @@ -1,6 +1,39 @@ -import { getNodeIDs } from './filter-nodes'; +import getFormattedNodes, { getNodeIDs } from './filter-nodes'; +import { mockState } from '../../utils/state.mock'; +import { getGroupedNodes } from '../../selectors/nodes'; describe('filter-nodes', () => { + describe('getFormattedNodes', () => { + const nodes = getGroupedNodes(mockState.lorem); + const searchTerm = 'e'; + const { formattedNodes, nodeIDs } = getFormattedNodes(nodes, searchTerm); + const nodeList = Object.keys(formattedNodes).reduce( + (names, key) => names.concat(formattedNodes[key]), + [] + ); + + test.each(nodeList.map(node => node.name))( + `node name "%s" contains search term "${searchTerm}"`, + name => { + expect(name).toEqual(expect.stringMatching(searchTerm)); + } + ); + + test.each(nodeList.map(node => node.highlightedLabel))( + `node label "%s" contains highlighted search term "${searchTerm}"`, + name => { + expect(name).toEqual(expect.stringMatching(`${searchTerm}`)); + } + ); + + test.each(nodeIDs)( + `node ID "%s" contains search term "${searchTerm}"`, + nodeID => { + expect(nodeID).toEqual(expect.stringMatching(searchTerm)); + } + ); + }); + describe('getNodeIDs', () => { const generateNodes = (type, count) => Array.from(new Array(count)).map((d, i) => ({ From 961e0d87e08347d019ad27c1f86f0a532568d04d Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Tue, 28 Apr 2020 15:25:35 +0100 Subject: [PATCH 05/13] Add tests for highlightMatch --- src/components/node-list/filter-nodes.test.js | 90 +++++++++++++------ 1 file changed, 64 insertions(+), 26 deletions(-) diff --git a/src/components/node-list/filter-nodes.test.js b/src/components/node-list/filter-nodes.test.js index 7f14b67135..7c7a3b3a2f 100644 --- a/src/components/node-list/filter-nodes.test.js +++ b/src/components/node-list/filter-nodes.test.js @@ -1,37 +1,44 @@ -import getFormattedNodes, { getNodeIDs } from './filter-nodes'; +import getFormattedNodes, { getNodeIDs, highlightMatch } from './filter-nodes'; import { mockState } from '../../utils/state.mock'; import { getGroupedNodes } from '../../selectors/nodes'; +const ungroupNodes = groupedNodes => + Object.keys(groupedNodes).reduce( + (names, key) => names.concat(groupedNodes[key]), + [] + ); + describe('filter-nodes', () => { describe('getFormattedNodes', () => { const nodes = getGroupedNodes(mockState.lorem); const searchTerm = 'e'; const { formattedNodes, nodeIDs } = getFormattedNodes(nodes, searchTerm); - const nodeList = Object.keys(formattedNodes).reduce( - (names, key) => names.concat(formattedNodes[key]), - [] - ); - - test.each(nodeList.map(node => node.name))( - `node name "%s" contains search term "${searchTerm}"`, - name => { - expect(name).toEqual(expect.stringMatching(searchTerm)); - } - ); - - test.each(nodeList.map(node => node.highlightedLabel))( - `node label "%s" contains highlighted search term "${searchTerm}"`, - name => { - expect(name).toEqual(expect.stringMatching(`${searchTerm}`)); - } - ); - - test.each(nodeIDs)( - `node ID "%s" contains search term "${searchTerm}"`, - nodeID => { - expect(nodeID).toEqual(expect.stringMatching(searchTerm)); - } - ); + const nodeList = ungroupNodes(formattedNodes); + + describe('formattedNodes', () => { + test.each(nodeList.map(node => node.name))( + `node name "%s" contains search term "${searchTerm}"`, + name => { + expect(name).toEqual(expect.stringMatching(searchTerm)); + } + ); + + test.each(nodeList.map(node => node.highlightedLabel))( + `node label "%s" contains highlighted search term "${searchTerm}"`, + name => { + expect(name).toEqual(expect.stringMatching(`${searchTerm}`)); + } + ); + }); + + describe('nodeIDs', () => { + test.each(nodeIDs)( + `node ID "%s" contains search term "${searchTerm}"`, + nodeID => { + expect(nodeID).toEqual(expect.stringMatching(searchTerm)); + } + ); + }); }); describe('getNodeIDs', () => { @@ -55,4 +62,35 @@ describe('filter-nodes', () => { expect(nodeIDs).toContain('parameters1'); }); }); + + describe('highlightMatch', () => { + const nodes = getGroupedNodes(mockState.lorem); + const searchTerm = 'e'; + const formattedNodes = highlightMatch(nodes, searchTerm); + const nodeList = ungroupNodes(formattedNodes); + + describe(`nodes which match the search term "${searchTerm}"`, () => { + const matchingNodeList = nodeList.filter(node => + node.name.includes(searchTerm) + ); + test.each(matchingNodeList.map(node => node.highlightedLabel))( + `node label "%s" contains highlighted search term "${searchTerm}"`, + name => { + expect(name).toEqual(expect.stringMatching(`${searchTerm}`)); + } + ); + }); + + describe(`nodes which do not match the search term "${searchTerm}"`, () => { + const notMatchingNodeList = nodeList.filter( + node => !node.name.includes(searchTerm) + ); + test.each(notMatchingNodeList.map(node => node.highlightedLabel))( + `node label "%s" does not contain ""`, + name => { + expect(name).not.toEqual(expect.stringMatching(``)); + } + ); + }); + }); }); From 3c156117949119212742feafde660cb717068a78 Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Tue, 28 Apr 2020 15:33:50 +0100 Subject: [PATCH 06/13] Add tests for nodeMatchesSearch --- src/components/node-list/filter-nodes.test.js | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/components/node-list/filter-nodes.test.js b/src/components/node-list/filter-nodes.test.js index 7c7a3b3a2f..3d7a3cd037 100644 --- a/src/components/node-list/filter-nodes.test.js +++ b/src/components/node-list/filter-nodes.test.js @@ -1,4 +1,8 @@ -import getFormattedNodes, { getNodeIDs, highlightMatch } from './filter-nodes'; +import getFormattedNodes, { + getNodeIDs, + highlightMatch, + nodeMatchesSearch +} from './filter-nodes'; import { mockState } from '../../utils/state.mock'; import { getGroupedNodes } from '../../selectors/nodes'; @@ -93,4 +97,27 @@ describe('filter-nodes', () => { ); }); }); + + describe('nodeMatchesSearch', () => { + const node = { name: 'qwertyuiop' }; + + it('returns true if the node name matches the search', () => { + expect(nodeMatchesSearch(node, 'qwertyuiop')).toBe(true); + expect(nodeMatchesSearch(node, 'qwe')).toBe(true); + expect(nodeMatchesSearch(node, 'p')).toBe(true); + }); + + it('returns true if the search is falsey', () => { + expect(nodeMatchesSearch(node, '')).toBe(true); + expect(nodeMatchesSearch(node, null)).toBe(true); + expect(nodeMatchesSearch(node, undefined)).toBe(true); + }); + + it('returns false if the node name does not match the search', () => { + expect(nodeMatchesSearch(node, 'a')).toBe(false); + expect(nodeMatchesSearch(node, 'qwe ')).toBe(false); + expect(nodeMatchesSearch(node, ' ')).toBe(false); + expect(nodeMatchesSearch(node, '_')).toBe(false); + }); + }); }); From e878fa85a04266e6c95a12a271fcbc715c30c1f1 Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Tue, 28 Apr 2020 15:57:28 +0100 Subject: [PATCH 07/13] Add tests for filterNodes --- src/components/node-list/filter-nodes.test.js | 43 ++++++++++++++++--- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/src/components/node-list/filter-nodes.test.js b/src/components/node-list/filter-nodes.test.js index 3d7a3cd037..b21349ac21 100644 --- a/src/components/node-list/filter-nodes.test.js +++ b/src/components/node-list/filter-nodes.test.js @@ -1,7 +1,8 @@ import getFormattedNodes, { getNodeIDs, highlightMatch, - nodeMatchesSearch + nodeMatchesSearch, + filterNodes } from './filter-nodes'; import { mockState } from '../../utils/state.mock'; import { getGroupedNodes } from '../../selectors/nodes'; @@ -68,7 +69,7 @@ describe('filter-nodes', () => { }); describe('highlightMatch', () => { - const nodes = getGroupedNodes(mockState.lorem); + const nodes = getGroupedNodes(mockState.animals); const searchTerm = 'e'; const formattedNodes = highlightMatch(nodes, searchTerm); const nodeList = ungroupNodes(formattedNodes); @@ -79,8 +80,8 @@ describe('filter-nodes', () => { ); test.each(matchingNodeList.map(node => node.highlightedLabel))( `node label "%s" contains highlighted search term "${searchTerm}"`, - name => { - expect(name).toEqual(expect.stringMatching(`${searchTerm}`)); + label => { + expect(label).toEqual(expect.stringMatching(`${searchTerm}`)); } ); }); @@ -91,8 +92,8 @@ describe('filter-nodes', () => { ); test.each(notMatchingNodeList.map(node => node.highlightedLabel))( `node label "%s" does not contain ""`, - name => { - expect(name).not.toEqual(expect.stringMatching(``)); + label => { + expect(label).not.toEqual(expect.stringMatching(``)); } ); }); @@ -120,4 +121,34 @@ describe('filter-nodes', () => { expect(nodeMatchesSearch(node, '_')).toBe(false); }); }); + + describe('filterNodes', () => { + const nodes = getGroupedNodes(mockState.animals); + const searchTerm = 'a'; + const filteredNodes = filterNodes(nodes, searchTerm); + const nodeList = ungroupNodes(filteredNodes); + const notMatchingNodeList = ungroupNodes(nodes).filter( + node => !node.name.includes(searchTerm) + ); + + describe('nodes which match the search term', () => { + test.each(nodeList.map(node => node.name))( + `node name "%s" should contain search term "${searchTerm}"`, + name => { + expect(name).toEqual(expect.stringMatching(searchTerm)); + } + ); + }); + + describe('nodes which do not match the search term', () => { + test.each(notMatchingNodeList.map(node => node.id))( + `filtered node list should not contain a node with id "%s"`, + nodeID => { + expect(nodeList.map(node => node.id)).not.toContain( + expect.stringMatching(searchTerm) + ); + } + ); + }); + }); }); From 124530e05d360ea000e4d8f50c9dd38e5a237baf Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Tue, 28 Apr 2020 16:08:52 +0100 Subject: [PATCH 08/13] Add tests for NodeListSearch --- src/components/node-list/node-list-search.js | 5 ++-- .../node-list/node-list-search.test.js | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 src/components/node-list/node-list-search.test.js diff --git a/src/components/node-list/node-list-search.js b/src/components/node-list/node-list-search.js index bcba5887f5..d37361b0d8 100644 --- a/src/components/node-list/node-list-search.js +++ b/src/components/node-list/node-list-search.js @@ -2,7 +2,6 @@ import React from 'react'; import SearchBar from '@quantumblack/kedro-ui/lib/components/search-bar'; import utils from '@quantumblack/kedro-ui/lib/utils'; import { connect } from 'react-redux'; -const { handleKeyEvent } = utils; /** * Handle Node List Search @@ -10,13 +9,13 @@ const { handleKeyEvent } = utils; * @param {string} searchValue Search text * @param {string} theme Light/dark theme for Kedro-UI component */ -const NodeListSearch = ({ onUpdateSearchValue, searchValue, theme }) => { +export const NodeListSearch = ({ onUpdateSearchValue, searchValue, theme }) => { /** * Listen for keyboard events, and trigger relevant actions * @param {number} keyCode The key event keycode */ const handleKeyDown = event => { - handleKeyEvent(event.keyCode, { + utils.handleKeyEvent(event.keyCode, { escape: onUpdateSearchValue.bind(this, '') }); }; diff --git a/src/components/node-list/node-list-search.test.js b/src/components/node-list/node-list-search.test.js new file mode 100644 index 0000000000..715dd5f883 --- /dev/null +++ b/src/components/node-list/node-list-search.test.js @@ -0,0 +1,26 @@ +import React from 'react'; +import { NodeListSearch, mapStateToProps } from './node-list-search'; +import { mockState, setup } from '../../utils/state.mock'; + +describe('NodeListSearch', () => { + it('renders without crashing', () => { + const wrapper = setup.shallow(NodeListSearch); + const search = wrapper.find('.pipeline-nodelist-search'); + expect(search.length).toBe(1); + }); + + it('handles escape key events', () => { + const onUpdateSearchValue = jest.fn(); + const wrapper = setup.shallow(NodeListSearch, { onUpdateSearchValue }); + const search = wrapper.find('.pipeline-nodelist-search'); + search.simulate('keydown', { keyCode: 27 }); + expect(onUpdateSearchValue.mock.calls.length).toBe(1); + }); + + it('maps state to props', () => { + const expectedResult = { + theme: expect.any(String) + }; + expect(mapStateToProps(mockState.lorem)).toEqual(expectedResult); + }); +}); From 6d533de730886899c6d9a1e57336935f7c493603 Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Tue, 28 Apr 2020 16:14:56 +0100 Subject: [PATCH 09/13] Remove unused variable --- src/components/node-list/node-list-search.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/node-list/node-list-search.test.js b/src/components/node-list/node-list-search.test.js index 715dd5f883..ee9d0fdae2 100644 --- a/src/components/node-list/node-list-search.test.js +++ b/src/components/node-list/node-list-search.test.js @@ -1,4 +1,3 @@ -import React from 'react'; import { NodeListSearch, mapStateToProps } from './node-list-search'; import { mockState, setup } from '../../utils/state.mock'; From d3f2f80d7bacd341420486d7b651168a072a2570 Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Tue, 28 Apr 2020 16:23:23 +0100 Subject: [PATCH 10/13] Add tests for NodeListToggleAll --- src/components/node-list/node-list-toggle.js | 2 +- .../node-list/node-list-toggle.test.js | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 src/components/node-list/node-list-toggle.test.js diff --git a/src/components/node-list/node-list-toggle.js b/src/components/node-list/node-list-toggle.js index a45849044d..8c8913560f 100644 --- a/src/components/node-list/node-list-toggle.js +++ b/src/components/node-list/node-list-toggle.js @@ -2,7 +2,7 @@ import React from 'react'; import { connect } from 'react-redux'; import { toggleNodesDisabled } from '../../actions/nodes'; -const NodeListToggleAll = ({ onToggleNodesDisabled, nodeIDs }) => ( +export const NodeListToggleAll = ({ onToggleNodesDisabled, nodeIDs }) => (

All Elements

diff --git a/src/components/node-list/node-list-toggle.test.js b/src/components/node-list/node-list-toggle.test.js new file mode 100644 index 0000000000..00adb2d503 --- /dev/null +++ b/src/components/node-list/node-list-toggle.test.js @@ -0,0 +1,30 @@ +import { NodeListToggleAll, mapDispatchToProps } from './node-list-toggle'; +import { setup } from '../../utils/state.mock'; + +describe('NodeListToggleAll', () => { + it('renders without crashing', () => { + const wrapper = setup.shallow(NodeListToggleAll); + expect(wrapper.find('.pipeline-nodelist__toggle').length).toBe(1); + }); + + it('handles button click events', () => { + const onToggleNodesDisabled = jest.fn(); + const wrapper = setup.shallow(NodeListToggleAll, { onToggleNodesDisabled }); + const button = wrapper.find('.pipeline-nodelist__toggle__button').first(); + button.simulate('click'); + expect(onToggleNodesDisabled.mock.calls.length).toBe(1); + }); + + describe('maps dispatch to props', () => { + it('toggles nodes disabled', () => { + const nodeIDs = ['123']; + const dispatch = jest.fn(); + mapDispatchToProps(dispatch).onToggleNodesDisabled(nodeIDs, true); + expect(dispatch.mock.calls[0][0]).toEqual({ + nodeIDs, + isDisabled: true, + type: 'TOGGLE_NODES_DISABLED' + }); + }); + }); +}); From 3a7ba4697a662db4d1cb723ae8d94898323ccaaa Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Wed, 29 Apr 2020 12:16:55 +0100 Subject: [PATCH 11/13] Rename function/variable to improve clarity --- src/components/node-list/filter-nodes.js | 8 ++++---- src/components/node-list/filter-nodes.test.js | 6 +++--- src/components/node-list/index.js | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/components/node-list/filter-nodes.js b/src/components/node-list/filter-nodes.js index b68285921a..8998673736 100644 --- a/src/components/node-list/filter-nodes.js +++ b/src/components/node-list/filter-nodes.js @@ -63,18 +63,18 @@ export const filterNodes = (nodes, searchValue) => { }; /** - * Return highlighted/formatted nodes, and filtered node IDs + * Return filtered/highlighted nodes, and filtered node IDs * @param {object} nodes Grouped lists of nodes * @param {string} searchValue Search term * @return {object} Grouped nodes, and node IDs */ -const getFormattedNodes = (nodes, searchValue) => { +const getFilteredNodes = (nodes, searchValue) => { const filteredNodes = filterNodes(nodes, searchValue); return { - formattedNodes: highlightMatch(filteredNodes, searchValue), + filteredNodes: highlightMatch(filteredNodes, searchValue), nodeIDs: getNodeIDs(filteredNodes) }; }; -export default getFormattedNodes; +export default getFilteredNodes; diff --git a/src/components/node-list/filter-nodes.test.js b/src/components/node-list/filter-nodes.test.js index b21349ac21..35a09d57e2 100644 --- a/src/components/node-list/filter-nodes.test.js +++ b/src/components/node-list/filter-nodes.test.js @@ -1,4 +1,4 @@ -import getFormattedNodes, { +import getFilteredNodes, { getNodeIDs, highlightMatch, nodeMatchesSearch, @@ -14,10 +14,10 @@ const ungroupNodes = groupedNodes => ); describe('filter-nodes', () => { - describe('getFormattedNodes', () => { + describe('getFilteredNodes', () => { const nodes = getGroupedNodes(mockState.lorem); const searchTerm = 'e'; - const { formattedNodes, nodeIDs } = getFormattedNodes(nodes, searchTerm); + const { formattedNodes, nodeIDs } = getFilteredNodes(nodes, searchTerm); const nodeList = ungroupNodes(formattedNodes); describe('formattedNodes', () => { diff --git a/src/components/node-list/index.js b/src/components/node-list/index.js index 543ae034ee..ae88f048a6 100644 --- a/src/components/node-list/index.js +++ b/src/components/node-list/index.js @@ -2,7 +2,7 @@ import React, { useState } from 'react'; import { connect } from 'react-redux'; import { Scrollbars } from 'react-custom-scrollbars'; import { getGroupedNodes } from '../../selectors/nodes'; -import getFormattedNodes from './filter-nodes'; +import getFilteredNodes from './filter-nodes'; import NodeListSearch from './node-list-search'; import NodeListToggleAll from './node-list-toggle'; import NodeListGroups from './node-list-groups'; @@ -13,7 +13,7 @@ import './styles/node-list.css'; */ const NodeList = ({ nodes }) => { const [searchValue, updateSearchValue] = useState(''); - const { formattedNodes, nodeIDs } = getFormattedNodes(nodes, searchValue); + const { filteredNodes, nodeIDs } = getFilteredNodes(nodes, searchValue); return ( @@ -27,7 +27,7 @@ const NodeList = ({ nodes }) => { autoHide hideTracksWhenNotNeeded> - + ); From 02a724dd57501b88f7e638fc34b6d1089294523b Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Wed, 29 Apr 2020 13:08:51 +0100 Subject: [PATCH 12/13] Fix renamed variables that I missed in prev commit --- src/components/node-list/filter-nodes.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/node-list/filter-nodes.test.js b/src/components/node-list/filter-nodes.test.js index 35a09d57e2..06717a8644 100644 --- a/src/components/node-list/filter-nodes.test.js +++ b/src/components/node-list/filter-nodes.test.js @@ -17,10 +17,10 @@ describe('filter-nodes', () => { describe('getFilteredNodes', () => { const nodes = getGroupedNodes(mockState.lorem); const searchTerm = 'e'; - const { formattedNodes, nodeIDs } = getFilteredNodes(nodes, searchTerm); - const nodeList = ungroupNodes(formattedNodes); + const { filteredNodes, nodeIDs } = getFilteredNodes(nodes, searchTerm); + const nodeList = ungroupNodes(filteredNodes); - describe('formattedNodes', () => { + describe('filteredNodes', () => { test.each(nodeList.map(node => node.name))( `node name "%s" contains search term "${searchTerm}"`, name => { From 8ae9b8d42a12d63a1db218ee51b756e9546315e4 Mon Sep 17 00:00:00 2001 From: Richard Westenra Date: Wed, 29 Apr 2020 13:15:23 +0100 Subject: [PATCH 13/13] Use selector for filter-nodes calculations this will memoize the result to prevent it being run unnecessarily --- src/components/node-list/filter-nodes.js | 18 +++++--- src/components/node-list/filter-nodes.test.js | 44 +++++++++---------- src/components/node-list/index.js | 2 +- 3 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/components/node-list/filter-nodes.js b/src/components/node-list/filter-nodes.js index 8998673736..fd42a46a78 100644 --- a/src/components/node-list/filter-nodes.js +++ b/src/components/node-list/filter-nodes.js @@ -1,3 +1,4 @@ +import { createSelector } from 'reselect'; import utils from '@quantumblack/kedro-ui/lib/utils'; const { escapeRegExp, getHighlightedText } = utils; @@ -68,13 +69,16 @@ export const filterNodes = (nodes, searchValue) => { * @param {string} searchValue Search term * @return {object} Grouped nodes, and node IDs */ -const getFilteredNodes = (nodes, searchValue) => { - const filteredNodes = filterNodes(nodes, searchValue); +const getFilteredNodes = createSelector( + [state => state.nodes, state => state.searchValue], + (nodes, searchValue) => { + const filteredNodes = filterNodes(nodes, searchValue); - return { - filteredNodes: highlightMatch(filteredNodes, searchValue), - nodeIDs: getNodeIDs(filteredNodes) - }; -}; + return { + filteredNodes: highlightMatch(filteredNodes, searchValue), + nodeIDs: getNodeIDs(filteredNodes) + }; + } +); export default getFilteredNodes; diff --git a/src/components/node-list/filter-nodes.test.js b/src/components/node-list/filter-nodes.test.js index 06717a8644..c789e6a0c5 100644 --- a/src/components/node-list/filter-nodes.test.js +++ b/src/components/node-list/filter-nodes.test.js @@ -16,31 +16,31 @@ const ungroupNodes = groupedNodes => describe('filter-nodes', () => { describe('getFilteredNodes', () => { const nodes = getGroupedNodes(mockState.lorem); - const searchTerm = 'e'; - const { filteredNodes, nodeIDs } = getFilteredNodes(nodes, searchTerm); + const searchValue = 'e'; + const { filteredNodes, nodeIDs } = getFilteredNodes({ nodes, searchValue }); const nodeList = ungroupNodes(filteredNodes); describe('filteredNodes', () => { test.each(nodeList.map(node => node.name))( - `node name "%s" contains search term "${searchTerm}"`, + `node name "%s" contains search term "${searchValue}"`, name => { - expect(name).toEqual(expect.stringMatching(searchTerm)); + expect(name).toEqual(expect.stringMatching(searchValue)); } ); test.each(nodeList.map(node => node.highlightedLabel))( - `node label "%s" contains highlighted search term "${searchTerm}"`, + `node label "%s" contains highlighted search term "${searchValue}"`, name => { - expect(name).toEqual(expect.stringMatching(`${searchTerm}`)); + expect(name).toEqual(expect.stringMatching(`${searchValue}`)); } ); }); describe('nodeIDs', () => { test.each(nodeIDs)( - `node ID "%s" contains search term "${searchTerm}"`, + `node ID "%s" contains search term "${searchValue}"`, nodeID => { - expect(nodeID).toEqual(expect.stringMatching(searchTerm)); + expect(nodeID).toEqual(expect.stringMatching(searchValue)); } ); }); @@ -70,25 +70,25 @@ describe('filter-nodes', () => { describe('highlightMatch', () => { const nodes = getGroupedNodes(mockState.animals); - const searchTerm = 'e'; - const formattedNodes = highlightMatch(nodes, searchTerm); + const searchValue = 'e'; + const formattedNodes = highlightMatch(nodes, searchValue); const nodeList = ungroupNodes(formattedNodes); - describe(`nodes which match the search term "${searchTerm}"`, () => { + describe(`nodes which match the search term "${searchValue}"`, () => { const matchingNodeList = nodeList.filter(node => - node.name.includes(searchTerm) + node.name.includes(searchValue) ); test.each(matchingNodeList.map(node => node.highlightedLabel))( - `node label "%s" contains highlighted search term "${searchTerm}"`, + `node label "%s" contains highlighted search term "${searchValue}"`, label => { - expect(label).toEqual(expect.stringMatching(`${searchTerm}`)); + expect(label).toEqual(expect.stringMatching(`${searchValue}`)); } ); }); - describe(`nodes which do not match the search term "${searchTerm}"`, () => { + describe(`nodes which do not match the search term "${searchValue}"`, () => { const notMatchingNodeList = nodeList.filter( - node => !node.name.includes(searchTerm) + node => !node.name.includes(searchValue) ); test.each(notMatchingNodeList.map(node => node.highlightedLabel))( `node label "%s" does not contain ""`, @@ -124,18 +124,18 @@ describe('filter-nodes', () => { describe('filterNodes', () => { const nodes = getGroupedNodes(mockState.animals); - const searchTerm = 'a'; - const filteredNodes = filterNodes(nodes, searchTerm); + const searchValue = 'a'; + const filteredNodes = filterNodes(nodes, searchValue); const nodeList = ungroupNodes(filteredNodes); const notMatchingNodeList = ungroupNodes(nodes).filter( - node => !node.name.includes(searchTerm) + node => !node.name.includes(searchValue) ); describe('nodes which match the search term', () => { test.each(nodeList.map(node => node.name))( - `node name "%s" should contain search term "${searchTerm}"`, + `node name "%s" should contain search term "${searchValue}"`, name => { - expect(name).toEqual(expect.stringMatching(searchTerm)); + expect(name).toEqual(expect.stringMatching(searchValue)); } ); }); @@ -145,7 +145,7 @@ describe('filter-nodes', () => { `filtered node list should not contain a node with id "%s"`, nodeID => { expect(nodeList.map(node => node.id)).not.toContain( - expect.stringMatching(searchTerm) + expect.stringMatching(searchValue) ); } ); diff --git a/src/components/node-list/index.js b/src/components/node-list/index.js index ae88f048a6..c0824c8d0b 100644 --- a/src/components/node-list/index.js +++ b/src/components/node-list/index.js @@ -13,7 +13,7 @@ import './styles/node-list.css'; */ const NodeList = ({ nodes }) => { const [searchValue, updateSearchValue] = useState(''); - const { filteredNodes, nodeIDs } = getFilteredNodes(nodes, searchValue); + const { filteredNodes, nodeIDs } = getFilteredNodes({ nodes, searchValue }); return (