From 33f5d99f241a6b33d8d661f31826039cd4a3c47c Mon Sep 17 00:00:00 2001 From: Rashida Kanchwala Date: Wed, 26 Jan 2022 15:56:27 +0000 Subject: [PATCH 01/18] WIP Signed-off-by: Rashida Kanchwala --- .eslintrc.json | 2 +- src/components/dropdown/dropdown-renderer.js | 203 ++++++++++ src/components/dropdown/dropdown.js | 374 ++++++++++++++++++ src/components/dropdown/dropdown.scss | 110 ++++++ src/components/dropdown/dropdown.test.js | 0 src/components/dropdown/event-controller.js | 43 ++ src/components/dropdown/index.js | 3 + src/components/pipeline-list/pipeline-list.js | 2 +- .../pipeline-list/pipeline-list.scss | 37 -- src/styles/_extends.scss | 25 ++ src/utils/key-events.js | 2 + 11 files changed, 762 insertions(+), 39 deletions(-) create mode 100644 src/components/dropdown/dropdown-renderer.js create mode 100644 src/components/dropdown/dropdown.js create mode 100644 src/components/dropdown/dropdown.scss create mode 100644 src/components/dropdown/dropdown.test.js create mode 100644 src/components/dropdown/event-controller.js create mode 100644 src/components/dropdown/index.js diff --git a/.eslintrc.json b/.eslintrc.json index 918e5b29f5..2a0eb7ba5d 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -4,7 +4,7 @@ "curly": ["error"], "valid-typeof": ["error"], "camelcase": "error", - "id-length": ["error", { "min": 3, "exceptions": ["_","a","b","d","e","i","j","k","x","y","id","el","pi","PI"] }], + "id-length": ["error", { "min": 3, "exceptions": ["_","a","b","d","e","i","j","k","x","y","id","el","pi","PI","up"] }], "no-var": ["error"], "lines-between-class-members": ["error", "always"] } diff --git a/src/components/dropdown/dropdown-renderer.js b/src/components/dropdown/dropdown-renderer.js new file mode 100644 index 0000000000..bf68c30fe9 --- /dev/null +++ b/src/components/dropdown/dropdown-renderer.js @@ -0,0 +1,203 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import classnames from 'classnames'; +import handleKeyEvent from '../../utils/key-events'; +import uniqueId from 'lodash/uniqueId'; + +/** + * Renderer for the Dropdown component + */ +const DropdownRenderer = ({ + children, + defaultText, + disabled, + focusedOption, + handleRef, + onLabelClicked, + onOptionSelected, + onSelectChanged, + open, + selectedOption, + title, + width, +}) => { + const wrapperClasses = classnames('kedro', 'kui-dropdown', { + 'kui-dropdown--open': open, + }); + let optionIndex = 0; + + /** + * Clone a React element and extend with extra props tieing it to a new scope + */ + const _extendMenuOption = (element, id, index) => { + const extraProps = { + id, + onSelected: onOptionSelected, + focused: focusedOption === index, + selected: + selectedOption.id === id || + (!selectedOption.id && element.props.selected), + }; + optionIndex += 1; + + return React.cloneElement(element, extraProps); + }; + + /** + * Handle keyboard events + * @param {Object} e - The key event object + */ + const _handleKeyDown = (e) => { + if (open) { + handleKeyEvent(e.keyCode, { + escape: onLabelClicked, + up: onSelectChanged.bind(this, -1), + down: onSelectChanged.bind(this, 1), + }); + } else { + handleKeyEvent(e.keyCode, { + up: onLabelClicked, + down: onLabelClicked, + }); + } + // Prevent the page from scrolling etc when using the dropdown: + handleKeyEvent(e.keyCode)('escape, up, down', () => e.preventDefault()); + }; + + const childElements = React.Children.toArray(children); + const sectionWrapRequired = + childElements[0] && typeof childElements[0].type === 'function'; + + // create options node + // we may have a plain array of Menu Options, in which case we'll wrap it with a section + // an array of sections, each containing an array of Menu Options + // sections may contain headings, which are defined as spans + const options = React.Children.map(childElements, (child, i) => { + switch (child.type) { + case 'section': + // one level of sections to iterate before we get to the Menu Options + return ( +
+ {React.Children.map(child.props.children, (sectionChild, j) => { + switch (sectionChild.type) { + case 'span': + // Heading + return sectionChild; + default: + // Menu Option + return _extendMenuOption( + sectionChild, + `menu-option-${i}.${j}`, + optionIndex + ); + } + })} +
+ ); + case 'span': + // Heading + return child; + default: + // Menu Option + return _extendMenuOption(child, `menu-option-${i}`, optionIndex); + } + }); + + const optionsNode = sectionWrapRequired ? ( +
{options}
+ ) : ( + options + ); + + return ( +
+ +
{optionsNode}
+
+ ); +}; + +DropdownRenderer.defaultProps = { + children: null, + defaultText: 'Please select...', + disabled: false, + focusedOption: null, + handleRef: null, + onLabelClicked: null, + onOptionSelected: null, + onSelectChanged: null, + open: false, + selectedOption: null, + title: null, + width: 160, +}; + +DropdownRenderer.propTypes = { + /** + * Child items. The nodes which React will pass down, defined inside the DropdownRenderer tag. + */ + children: PropTypes.node, + /** + * Default text to show in a closed unselected state + */ + defaultText: PropTypes.string, + /** + * The index of the currently-focused menu option + */ + focusedOption: PropTypes.number, + /** + * Retrieve a reference to the dropdown DOM node + */ + handleRef: PropTypes.func, + /** + * Callback to be executed when the main label is clicked + */ + onLabelClicked: PropTypes.func, + /** + * Callback to be executed when an option is selected + */ + onOptionSelected: PropTypes.func, + /** + * Callback to be executed when the focused option changes + */ + onSelectChanged: PropTypes.func, + /** + * Whether the dropdown is in an open state + */ + open: PropTypes.bool, + /** + * An object containing selected option details. + * This will be created based on the id, primaryText, value of a selected Menu Option. + */ + selectedOption: PropTypes.shape({ + id: PropTypes.string, + label: PropTypes.string, + value: PropTypes.any, + }), + + /** + * Title text for native tooltip + */ + title: PropTypes.string, + /** + * The width for the component. Both the label and options are the same width + */ + width: PropTypes.number, +}; + +export default DropdownRenderer; diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js new file mode 100644 index 0000000000..9f0785b7f6 --- /dev/null +++ b/src/components/dropdown/dropdown.js @@ -0,0 +1,374 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { flatten, find, flow, isEqual, map } from 'lodash/fp'; +import 'what-input'; +import './dropdown.css'; +import DropdownRenderer from './dropdown-renderer'; +import EventController from './event-controller.js'; + +class Dropdown extends React.Component { + /** + * Create a new Dropdown + * @param {Object} props + */ + constructor(props) { + super(props); + + this.displayName = 'Dropdown'; + + // bind method scope + this._handleRef = this._handleRef.bind(this); + this._getOptionsList = this._getOptionsList.bind(this); + this._handleLabelClicked = this._handleLabelClicked.bind(this); + this._handleOptionSelected = this._handleOptionSelected.bind(this); + this._handleFocusChange = this._handleFocusChange.bind(this); + this._handleBodyClicked = this._handleBodyClicked.bind(this); + this.open = this.open.bind(this); + this.close = this.close.bind(this); + + this.state = { + focusedOption: null, + selectedOption: this._findSelectedOption(), + open: false, + }; + } + + /** + * React lifecycle method + * {@link https://facebook.github.io/react/docs/react-component.html#componentwillupdate} + * @param {Object} New component props + */ + componentDidUpdate(prevProps) { + if (this._childrenHaveChanged(prevProps)) { + this.setState({ + selectedOption: this._findSelectedOption(prevProps), + }); + } + } + + /** + * React lifecycle method + * {@link https://facebook.github.io/react/docs/react-component.html#componentwillunmount} + * @return {object} JSX for this component + */ + componentWillUnmount() { + EventController.removeBodyListeners(); + } + + /** + * Handler for closing a dropdown if a click occured outside the dropdown. + * @param {object} e - event object + */ + _handleBodyClicked(e) { + if (!this.dropdown.contains(e.target) && this.state.open) { + this.close(); + } + } + + /** + * Check whether new props contain updated children + * @param {Object} nextProps - New component props + * @return {Boolean} True if new children are different from current ones + */ + _childrenHaveChanged(nextProps) { + const children = [this.props, nextProps].map((props) => + React.Children.toArray(props.children) + ); + + return !isEqual(...children); + } + + /** + * Format the selected option props for adding to state + * @param {Object} props - Component props + * @return {Object} Selected option object for use in the state + */ + _findSelectedOption(props) { + const selectedOptionElement = this._findSelectedOptionElement(props); + + // check children for a selected option + if (selectedOptionElement) { + const { id, primaryText, value } = selectedOptionElement.props; + + return { + id, + label: primaryText, + value, + }; + } + + // otherwise, default to first + return { + id: null, + label: null, + value: null, + }; + } + + /** + * Find the selected option by traversing sections and MenuOptions + * @param {Object} props - Component props (optional) + * @return {Object} Selected option element + */ + _findSelectedOptionElement(props = this.props) { + const children = React.Children.toArray(props.children); + + if (!children.length) { + return null; + } + + // we may have an array of options + // or an array of sections, containing options + if (children[0].type === 'section') { + return flow( + map((x) => x.props.children), + flatten, + find((x) => x.props.selected) + )(children); + } + + return find((child) => child.props.selected)(children); + } + + /** + * Event handler which is fired when the label is clicked + */ + _handleLabelClicked() { + const { open } = this.state; + const { onOpened, onClosed } = this.props; + + let callback = null; + + // set callbacks, if defined + if (typeof onOpened === 'function' && !open) { + callback = onOpened; + } else if (typeof onClosed === 'function' && open) { + callback = onClosed; + } + + // remove or add the event listeners for + if (open) { + EventController.removeBodyListeners(); + } else { + EventController.addBodyListener(this._handleBodyClicked); + } + + this.setState({ open: !open }, callback); + this._focusLabel(); + } + + /** + * Sort, filter and flatten the list of children to retrieve just the MenuOptions, + * with any Sections removed. + * @return {Object} A flat list of MenuOptions + */ + _getOptionsList() { + /** + * Recurse through sections to retrieve a list of all MenuOptions + * @param {Object} previous The Options array as of the previous iteration + * @param {Object} current The current item (either a MenuOption or Section) + * @return {Object} The current state of the Options array + */ + const getSectionChildren = (previous, current) => { + if (current.props.primaryText) { + // MenuOption: Add to list + return previous.concat(current); + } + if (current.type === 'section') { + // Section: Keep recursing + return previous.concat( + current.props.children.reduce(getSectionChildren, []) + ); + } + return previous; + }; + + return React.Children.toArray(this.props.children).reduce( + getSectionChildren, + [] + ); + } + + /** + * Convenience method to return focus from an option to the label. + * This is particularly useful for screen-readers and keyboard users. + */ + _focusLabel() { + this.dropdown.querySelector('.kui-dropdown__label').focus(); + + this.setState({ + focusedOption: null, + }); + } + + /** + * When the focused option changes (e.g. via up/down keyboard controls), + * update the focusedOption index state and select the new one + * @param {number} direction - The direction that focus is travelling through the list: + * negative is up and positive is down. + */ + _handleFocusChange(direction) { + let { focusedOption } = this.state; + const optionsLength = this._getOptionsList().length; + + if (focusedOption === null) { + focusedOption = direction > 0 ? 0 : optionsLength - 1; + } else { + focusedOption += direction; + } + if (focusedOption >= optionsLength || focusedOption < 0) { + focusedOption = null; + } + + this.setState({ focusedOption }, () => { + // Focus either the button label or the active option. + // This is so screen-readers will follow the active element + const focusClass = + focusedOption !== null + ? '.kui-menu-option--focused' + : '.kui-dropdown__label'; + + this.dropdown.querySelector(focusClass).focus(); + }); + } + + /** + * Event handler which is fired when a child item is selected + */ + _handleOptionSelected(obj) { + const { label, id, value } = obj; + const { onChanged, onClosed } = this.props; + + // detect if the selected item has changed + const hasChanged = value !== this.state.selectedOption.value; + if (hasChanged) { + const selectedOption = { label, value, id }; + this.setState({ open: false, selectedOption }, () => { + if (typeof onChanged === 'function') { + onChanged(obj); + } + + if (typeof onClosed === 'function') { + onClosed(); + } + }); + } else { + this.setState({ open: false }, () => { + if (typeof onClosed === 'function') { + onClosed(); + } + }); + } + this._focusLabel(); + } + + /** + * Retrieve a reference to the dropdown DOM node (from the renderer component), + * and assign it to a class-wide variable property. + * @param {object} el - The ref for the Dropdown container node + */ + _handleRef(el) { + this.dropdown = el; + } + + /** + * API method to open the dropdown + */ + open() { + const { onOpened } = this.props; + + this.setState({ open: true }, () => { + this._focusLabel(); + if (typeof onOpened === 'function') { + onOpened(); + } + }); + + // add event listener to automatically close the dropdown + EventController.addBodyListener(this._handleBodyClicked); + } + + /** + * API method to close the dropdown + */ + close() { + const { onClosed } = this.props; + + this.setState({ open: false }, () => { + if (typeof onClosed === 'function') { + onClosed(); + } + }); + + // remove event listener + EventController.removeBodyListeners(); + } + + /** + * React lifecycle method + * {@link https://facebook.github.io/react/docs/react-component.html#render} + * @return {object} JSX for this component + */ + render() { + const { children, defaultText, width } = this.props; + const { open, focusedOption, selectedOption } = this.state; + + return ( + + {children} + + ); + } +} + +Dropdown.defaultProps = { + children: null, + defaultText: 'Please select...', + onChanged: null, + onClosed: null, + onOpened: null, + width: 160, +}; + +Dropdown.propTypes = { + /** + * Child items. The nodes which React will pass down, defined inside the DropdownRenderer tag + */ + children: PropTypes.node.isRequired, + /** + * Default text to show in a closed unselected state + */ + defaultText: PropTypes.string, + /** + * Whether to disable the dropdown + */ + disabled: PropTypes.bool, + /** + * Callback function to be executed when a menu item is clicked, other than the one currently selected. + */ + onChanged: PropTypes.func, + /** + * Callback to be executed after menu opens + */ + onOpened: PropTypes.func, + /** + * Callback to be executed after menu closed + */ + onClosed: PropTypes.func, + /** + * The width for the component. Both the label and options are the same width + */ + width: PropTypes.number, +}; + +export default Dropdown; diff --git a/src/components/dropdown/dropdown.scss b/src/components/dropdown/dropdown.scss new file mode 100644 index 0000000000..36886627b4 --- /dev/null +++ b/src/components/dropdown/dropdown.scss @@ -0,0 +1,110 @@ +@import '../../styles/extends'; +@import '../../styles/variables'; + +/** Variables */ + +/* the maximum dropdown before a scrollbar is required */ +$menu-max-height: 300px; + +/* the height of each item/row */ +$menu-item-height: 40px; + +/* padding at the top and bottom of each group of items */ +$menu-group-padding: 4px; + +/* the padding of each item (either side of the text ) */ +$menu-item-padding: 12px; + +.kui-theme--light { + --dropdown-shadow: #{rgba($color-dark, 0.03)}; + --dropdown-label-bg: #{$color-bg-light-1}; + --dropdown-border: #{rgba($color-dark, 0.12)}; + --dropdown-options-bg: #{$color-bg-light-3}; +} + +.kui-theme--dark { + --dropdown-shadow: #{rgba($color-dark, 0.1)}; + --dropdown-label-bg: #{$color-bg-dark-3}; + --dropdown-border: #{color-dark}; + --dropdown-options-bg: #{$color-bg-dark-1}; +} + +.kui-dropdown { + position: relative; + box-sizing: border-box; + display: block; + border: none; + box-shadow: 0 1px 1px var(--dropdown-shadow); + color: var(--color-text); + border: 1px solid var(--dropdown-border); +} + +.kui-dropdown--open { + position: relative; + + z-index: 9; +} + +.kui-dropdown__label { + position: relative; + z-index: 8; + + display: flex; + width: 100%; + align-items: center; + height: $menu-item-height; + padding: 0 $sidebar-padding / 1.6; // divide by 1.6em font-size on element + overflow: hidden; + border: none; + border-radius: 0; + box-shadow: none; + outline: 4px solid transparent; + font-family: inherit; + font-size: 1.6em; + letter-spacing: 0.1px; + line-height: 1.25em; + background: var(--dropdown-label-bg); + color: var(--color-text); + cursor: pointer; + user-select: none; + + &:focus { + &:focus { + outline: none; + box-shadow: 0 0 0 4px $color-primary inset; + + [data-whatinput='mouse'] & { + box-shadow: none; + } + } + } +} + +.kui-dropdown__options { + z-index: 2; // fix closing transition animation bug + background: var(--dropdown-options-bg); + border-top: none; + box-shadow: 0 0 2px rgba(black, 0.1); + position: absolute; + visibility: hidden; + width: 100%; + max-height: $menu-max-height; + overflow: auto; + margin: 0; + border-style: solid; + border-width: 1px 0 0; + transition: opacity ease 0.2s, transform ease 0.2s, visibility ease 0.2s; + transform: translateY(-10px); + opacity: 0; + + .kui-dropdown--open & { + visibility: visible; + transition: opacity ease 0.3s, transform ease 0.3s, visibility ease 0.3s; + opacity: 1; + transform: translateY(0); + } + + > section { + padding: 0; + } +} diff --git a/src/components/dropdown/dropdown.test.js b/src/components/dropdown/dropdown.test.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/components/dropdown/event-controller.js b/src/components/dropdown/event-controller.js new file mode 100644 index 0000000000..ea6ab4b0df --- /dev/null +++ b/src/components/dropdown/event-controller.js @@ -0,0 +1,43 @@ +/** + * Event Controller for managing listeners for the dropdown component. + * Handles adding and removing listeners attached to body of the document. + */ +class EventController { + /** + * Manages the attachment of the event listener when body is clicked. + * @param {object} eventHandler - event handler which will be added + */ + static addBodyListener(eventHandler) { + if (typeof window.__bodyEventHandlers === 'undefined') { + window.__bodyEventHandlers = []; + } + + // add event handler to the array attached to the windown so that it can be retrieved outside of component + window.__bodyEventHandlers.push(eventHandler); + // add the event handler to the body + document.body.addEventListener('click', eventHandler); + + // indicate that event listeners are attached + window.__bodyListenerAttached = true; + } + + /** + * Manages the removal of the event listeners when body is clicked - all event listeners added + * by dropdown components are removed. + * This method is static because it doesn't utilize 'this'. + */ + static removeBodyListeners() { + if (window.__bodyListenerAttached) { + // remove all event listeners attached to body + window.__bodyEventHandlers.forEach((handler) => { + document.body.removeEventListener('click', handler); + }); + + // indicate that no listeners are attached and reset the array + window.__bodyEventHandlers = []; + window.__bodyListenerAttached = false; + } + } +} + +export default EventController; diff --git a/src/components/dropdown/index.js b/src/components/dropdown/index.js new file mode 100644 index 0000000000..e55f5586f3 --- /dev/null +++ b/src/components/dropdown/index.js @@ -0,0 +1,3 @@ +import Dropdown from './dropdown'; + +export default Dropdown; diff --git a/src/components/pipeline-list/pipeline-list.js b/src/components/pipeline-list/pipeline-list.js index 607fd4aa07..4537a954c1 100644 --- a/src/components/pipeline-list/pipeline-list.js +++ b/src/components/pipeline-list/pipeline-list.js @@ -1,7 +1,7 @@ import React from 'react'; import classnames from 'classnames'; import { connect } from 'react-redux'; -import Dropdown from '@quantumblack/kedro-ui/lib/components/dropdown'; +import Dropdown from '../dropdown'; import MenuOption from '@quantumblack/kedro-ui/lib/components/menu-option'; import { loadPipelineData } from '../../actions/pipelines'; import { toggleFocusMode } from '../../actions'; diff --git a/src/components/pipeline-list/pipeline-list.scss b/src/components/pipeline-list/pipeline-list.scss index 0fa9abce9d..ec1a7bb44b 100644 --- a/src/components/pipeline-list/pipeline-list.scss +++ b/src/components/pipeline-list/pipeline-list.scss @@ -2,60 +2,23 @@ @import '../../styles/variables'; .kui-theme--light { - --pipeline-list-shadow: rgba(black, 0.03); - --pipeline-list-label-bg: #{$color-bg-light-1}; - --pipeline-list-options-bg: #{$color-bg-light-3}; --pipeline-list-option-border-width: 1px 0 0 2px; --pipeline-list-option-border-color: #{rgba(black, 0.06)}; --pipeline-list-option-bg-hover: #{rgba(white, 0.3)}; } .kui-theme--dark { - --pipeline-list-shadow: rgba(black, 0.1); - --pipeline-list-label-bg: #{$color-bg-dark-3}; - --pipeline-list-options-bg: #{$color-bg-dark-1}; --pipeline-list-option-border-width: 0 0 1px 2px; --pipeline-list-option-border-color: #{rgba(white, 0.06)}; --pipeline-list-option-bg-hover: #{rgba(white, 0.03)}; } .pipeline-list { - .kui-dropdown { - display: block; - border: none; - box-shadow: 0 1px 1px var(--pipeline-list-shadow) !important; - } - .kui-dropdown__label, .kui-menu-option { height: 58px; } - .kui-dropdown__label { - padding: 0 $sidebar-padding / 1.6; // divide by 1.6em font-size on element - background: var(--pipeline-list-label-bg); - - &:focus { - outline: none; - box-shadow: 0 0 0 4px $color-primary inset; - - [data-whatinput='mouse'] & { - box-shadow: none; - } - } - } - - .kui-dropdown__options { - z-index: 2; // fix closing transition animation bug - background: var(--pipeline-list-options-bg); - border-top: none; - box-shadow: 0 0 2px rgba(black, 0.1); - - > section { - padding: 0; - } - } - .kui-menu-option { padding: 0 $sidebar-padding 0 calc(#{$sidebar-padding} - 2px); border-color: var(--pipeline-list-option-border-color) transparent; diff --git a/src/styles/_extends.scss b/src/styles/_extends.scss index e7264c93d0..7405d87968 100644 --- a/src/styles/_extends.scss +++ b/src/styles/_extends.scss @@ -68,3 +68,28 @@ font-size: 14px; font-weight: normal; } + +%shadow-light { + box-shadow: 0 0 2px 0 rgba(0, 0, 0, 0.24), 0 0 2px 0 rgba(0, 0, 0, 0.08); +} + +%shadow-light--hover { + box-shadow: -2px 0 4px 0 rgba(0, 0, 0, 0.08), 2px 0 4px 0 rgba(0, 0, 0, 0.08), + 0 2px 4px 0 rgba(0, 0, 0, 0.12), 0 0 2px 0 rgba(0, 0, 0, 0.12); +} + +%shadow-dark { + box-shadow: 0 0 2px 0 rgba(0, 0, 0, 0.24), 0 0 2px 0 rgba(0, 0, 0, 0.12), + inset 0 1px 1px 0 rgba(255, 255, 255, 0.03); +} + +%shadow-dark--hover { + box-shadow: -2px 0 4px 0 rgba(0, 0, 0, 0.08), 2px 0 4px 0 rgba(0, 0, 0, 0.08), + 0 2px 4px 0 rgba(0, 0, 0, 0.12), 0 0 2px 0 rgba(0, 0, 0, 0.24), + 0 0 2px 0 rgba(0, 0, 0, 0.24), 0 0 2px 0 rgba(0, 0, 0, 0.12), + inset 0 1px 1px 0 rgba(255, 255, 255, 0.03); +} + +%shadow-light--active { + box-shadow: 0 0 2px 0 rgba(0, 0, 0, 0.24), 0 0 2px 0 rgba(0, 0, 0, 0.08); +} diff --git a/src/utils/key-events.js b/src/utils/key-events.js index 751b5700fa..6c46758ffd 100644 --- a/src/utils/key-events.js +++ b/src/utils/key-events.js @@ -1,6 +1,8 @@ // Keyboard character codes const KEYS = { 27: 'escape', + 38: 'up', + 40: 'down', }; /** From b3bcde56f2403f9fe2b7854e6cac558291a222f1 Mon Sep 17 00:00:00 2001 From: Rashida Kanchwala Date: Wed, 26 Jan 2022 18:15:23 +0000 Subject: [PATCH 02/18] Added menu option Signed-off-by: Rashida Kanchwala --- src/components/dropdown/dropdown.scss | 2 - src/components/menu-option/index.js | 3 + src/components/menu-option/menu-option.js | 104 ++++++++++++++++++ src/components/menu-option/menu-option.scss | 63 +++++++++++ .../menu-option/menu-option.test.js | 0 src/components/pipeline-list/pipeline-list.js | 4 +- .../pipeline-list/pipeline-list.scss | 17 --- 7 files changed, 171 insertions(+), 22 deletions(-) create mode 100644 src/components/menu-option/index.js create mode 100644 src/components/menu-option/menu-option.js create mode 100644 src/components/menu-option/menu-option.scss create mode 100644 src/components/menu-option/menu-option.test.js diff --git a/src/components/dropdown/dropdown.scss b/src/components/dropdown/dropdown.scss index 36886627b4..fd4660abe5 100644 --- a/src/components/dropdown/dropdown.scss +++ b/src/components/dropdown/dropdown.scss @@ -91,8 +91,6 @@ $menu-item-padding: 12px; max-height: $menu-max-height; overflow: auto; margin: 0; - border-style: solid; - border-width: 1px 0 0; transition: opacity ease 0.2s, transform ease 0.2s, visibility ease 0.2s; transform: translateY(-10px); opacity: 0; diff --git a/src/components/menu-option/index.js b/src/components/menu-option/index.js new file mode 100644 index 0000000000..407b645042 --- /dev/null +++ b/src/components/menu-option/index.js @@ -0,0 +1,3 @@ +import MenuOption from './menu-option'; + +export default MenuOption; diff --git a/src/components/menu-option/menu-option.js b/src/components/menu-option/menu-option.js new file mode 100644 index 0000000000..a4719f400d --- /dev/null +++ b/src/components/menu-option/menu-option.js @@ -0,0 +1,104 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import classnames from 'classnames'; +import handleKeyEvent from '../../utils/key-events'; +import './menu-option.css'; +/** +Generic Kedro Menu Option + */ +const MenuOption = ({ + className, + focused, + id, + onSelected, + primaryText, + selected, + value, +}) => { + const wrapperClasses = classnames('kedro', 'kui-menu-option', className, { + 'kui-menu-option--focused': focused, + 'kui-menu-option--selected': selected, + }); + + /** + * Event handler executed when the option is selected + * @param {object} e The event object + * @return {function} The event handler + */ + const _handleClicked = (e) => + onSelected({ + event: e, + id, + label: primaryText, + value, + }); + + /** + * Event handler executed when key events are triggered on the focused option + * @param {Object} e - The key event object + */ + const _handleKeyDown = (e) => + handleKeyEvent(e.keyCode)('enter, space', () => { + _handleClicked(e); + // Prevent the page from scrolling when selecting an item: + e.preventDefault(); + }); + + return ( +
+
+ {primaryText} +
+
+ ); +}; + +MenuOption.defaultProps = { + className: null, + focused: false, + id: null, + onSelected: null, + selected: false, + value: null, +}; + +MenuOption.propTypes = { + /** + * Container class + */ + className: PropTypes.string, + /** + * Whether the option is focused + */ + focused: PropTypes.bool, + /** + * A unique key for this element, which will be set by the parent menu component. + * This is used by the parent menu component to determine which option is selected. + */ + id: PropTypes.string, + /** + * A callback which is automatically implemented by a parent menu component + */ + onSelected: PropTypes.func, + /** + * The main label displayed + */ + primaryText: PropTypes.string.isRequired, + /** + * Whether the option is selected + */ + selected: PropTypes.bool, + /** + * The value to send to the parent menu component when this item is selected + */ + value: PropTypes.any, +}; + +export default MenuOption; diff --git a/src/components/menu-option/menu-option.scss b/src/components/menu-option/menu-option.scss new file mode 100644 index 0000000000..67da8d8850 --- /dev/null +++ b/src/components/menu-option/menu-option.scss @@ -0,0 +1,63 @@ +@import '../../styles/variables'; + +/** Variables */ + +/* the height of each item/row */ +$menu-item-height: 40px; + +/* padding at the top and bottom of each group of items */ +$menu-group-padding: 4px; + +/* the padding of each item (either side of the text ) */ +$menu-item-padding: 12px; + +.kui-theme--light { + --menu-option-border-width: 1px 0 0 2px; + --menu-option-border-color: #{rgba(black, 0.06)}; + --menu-option-bg-hover: #{rgba(white, 0.3)}; +} + +.kui-theme--dark { + --menu-option-border-width: 0 0 1px 2px; + --menu-option-border-color: #{rgba(white, 0.06)}; + --menu-option-bg-hover: #{rgba(white, 0.03)}; +} + +.kui-menu-option { + display: flex; + align-items: center; + box-sizing: border-box; + + height: $menu-item-height; + + padding: 0 $sidebar-padding 0 calc(#{$sidebar-padding} - 2px); + border-color: var(--menu-option-border-color) transparent; + border-style: solid; + border-width: var(--menu-option-border-width); + white-space: nowrap; + overflow: hidden; + cursor: pointer; + user-select: none; + outline: none; + + &.kui-menu-option--focused, + &:focus, + &:hover { + background: var(--menu-option-bg-hover); + } + + &:active { + border-left-color: $color-primary; + } +} + +.kui-menu-option__content { + position: relative; + display: flex; + align-items: center; + width: 100%; + text-overflow: ellipsis; + font-size: 1.6em; + letter-spacing: 0.1px; + line-height: 1.25; +} diff --git a/src/components/menu-option/menu-option.test.js b/src/components/menu-option/menu-option.test.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/components/pipeline-list/pipeline-list.js b/src/components/pipeline-list/pipeline-list.js index 4537a954c1..2469a2d5ba 100644 --- a/src/components/pipeline-list/pipeline-list.js +++ b/src/components/pipeline-list/pipeline-list.js @@ -2,7 +2,7 @@ import React from 'react'; import classnames from 'classnames'; import { connect } from 'react-redux'; import Dropdown from '../dropdown'; -import MenuOption from '@quantumblack/kedro-ui/lib/components/menu-option'; +import MenuOption from '../menu-option'; import { loadPipelineData } from '../../actions/pipelines'; import { toggleFocusMode } from '../../actions'; import './pipeline-list.css'; @@ -31,7 +31,6 @@ export const PipelineList = ({ disabled={!pipeline.ids.length} onOpened={() => onToggleOpen(true)} onClosed={() => onToggleOpen(false)} - theme={theme} width={null} onChanged={onUpdateActivePipeline} defaultText={ @@ -59,7 +58,6 @@ export const mapStateToProps = (state) => ({ asyncDataSource: state.dataSource === 'json', pipeline: state.pipeline, prettyName: state.prettyName, - theme: state.theme, }); export const mapDispatchToProps = (dispatch) => ({ diff --git a/src/components/pipeline-list/pipeline-list.scss b/src/components/pipeline-list/pipeline-list.scss index ec1a7bb44b..f4669c2b2c 100644 --- a/src/components/pipeline-list/pipeline-list.scss +++ b/src/components/pipeline-list/pipeline-list.scss @@ -18,21 +18,4 @@ .kui-menu-option { height: 58px; } - - .kui-menu-option { - padding: 0 $sidebar-padding 0 calc(#{$sidebar-padding} - 2px); - border-color: var(--pipeline-list-option-border-color) transparent; - border-style: solid; - border-width: var(--pipeline-list-option-border-width); - - &--focused, - &:focus, - &:hover { - background: var(--pipeline-list-option-bg-hover); - } - - &.pipeline-list__option--active { - border-left-color: $color-primary; - } - } } From ebed735f3bf3892fefd5927ec50b9c3110090e48 Mon Sep 17 00:00:00 2001 From: Rashida Kanchwala Date: Wed, 26 Jan 2022 18:21:58 +0000 Subject: [PATCH 03/18] Fixed styling Signed-off-by: Rashida Kanchwala --- src/components/menu-option/menu-option.scss | 4 ---- src/components/pipeline-list/pipeline-list.scss | 6 ++++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/menu-option/menu-option.scss b/src/components/menu-option/menu-option.scss index 67da8d8850..fd06a0c48c 100644 --- a/src/components/menu-option/menu-option.scss +++ b/src/components/menu-option/menu-option.scss @@ -45,10 +45,6 @@ $menu-item-padding: 12px; &:hover { background: var(--menu-option-bg-hover); } - - &:active { - border-left-color: $color-primary; - } } .kui-menu-option__content { diff --git a/src/components/pipeline-list/pipeline-list.scss b/src/components/pipeline-list/pipeline-list.scss index f4669c2b2c..9901c9e5c5 100644 --- a/src/components/pipeline-list/pipeline-list.scss +++ b/src/components/pipeline-list/pipeline-list.scss @@ -18,4 +18,10 @@ .kui-menu-option { height: 58px; } + + .kui-menu-option { + &.pipeline-list__option--active { + border-left-color: $color-primary; + } + } } From 156b147fa91485acdea462a02fa93ffcf1588040 Mon Sep 17 00:00:00 2001 From: Rashida Kanchwala Date: Thu, 27 Jan 2022 15:05:16 +0000 Subject: [PATCH 04/18] Added tests Signed-off-by: Rashida Kanchwala --- src/components/app/app.test.js | 2 +- src/components/dropdown/dropdown-renderer.js | 13 ++++-- src/components/dropdown/dropdown.js | 10 ++--- src/components/dropdown/dropdown.scss | 18 +++++--- src/components/dropdown/dropdown.test.js | 40 +++++++++++++++++ src/components/menu-option/menu-option.js | 8 ++-- src/components/menu-option/menu-option.scss | 6 +-- .../menu-option/menu-option.test.js | 45 +++++++++++++++++++ .../pipeline-list/pipeline-list.scss | 6 +-- .../pipeline-list/pipeline-list.test.js | 7 ++- 10 files changed, 126 insertions(+), 29 deletions(-) diff --git a/src/components/app/app.test.js b/src/components/app/app.test.js index e899ad7cad..2470ddfc21 100644 --- a/src/components/app/app.test.js +++ b/src/components/app/app.test.js @@ -95,7 +95,7 @@ describe('App', () => { const pipelineDropdown = container.querySelector('.pipeline-list'); const menuOption = within(pipelineDropdown).getByText(activePipeline.name); const pipelineDropdownLabel = pipelineDropdown.querySelector( - '.kui-dropdown__label > span:first-child' + '.dropdown__label > span:first-child' ); expect(pipelineDropdownLabel.innerHTML).toBe('Default'); fireEvent.click(menuOption); diff --git a/src/components/dropdown/dropdown-renderer.js b/src/components/dropdown/dropdown-renderer.js index bf68c30fe9..9cfc73b6b9 100644 --- a/src/components/dropdown/dropdown-renderer.js +++ b/src/components/dropdown/dropdown-renderer.js @@ -21,8 +21,9 @@ const DropdownRenderer = ({ title, width, }) => { - const wrapperClasses = classnames('kedro', 'kui-dropdown', { - 'kui-dropdown--open': open, + const wrapperClasses = classnames('kedro', 'dropdown', { + 'dropdown--open': open, + 'dropdown--disabled': disabled, }); let optionIndex = 0; @@ -122,12 +123,12 @@ const DropdownRenderer = ({ -
{optionsNode}
+
{optionsNode}
); }; @@ -156,6 +157,10 @@ DropdownRenderer.propTypes = { * Default text to show in a closed unselected state */ defaultText: PropTypes.string, + /** + * Whether to disable the dropdown + */ + disabled: PropTypes.bool, /** * The index of the currently-focused menu option */ diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js index 9f0785b7f6..a5c699a765 100644 --- a/src/components/dropdown/dropdown.js +++ b/src/components/dropdown/dropdown.js @@ -194,7 +194,7 @@ class Dropdown extends React.Component { * This is particularly useful for screen-readers and keyboard users. */ _focusLabel() { - this.dropdown.querySelector('.kui-dropdown__label').focus(); + this.dropdown.querySelector('.dropdown__label').focus(); this.setState({ focusedOption: null, @@ -224,9 +224,7 @@ class Dropdown extends React.Component { // Focus either the button label or the active option. // This is so screen-readers will follow the active element const focusClass = - focusedOption !== null - ? '.kui-menu-option--focused' - : '.kui-dropdown__label'; + focusedOption !== null ? '.menu-option--focused' : '.dropdown__label'; this.dropdown.querySelector(focusClass).focus(); }); @@ -310,12 +308,13 @@ class Dropdown extends React.Component { * @return {object} JSX for this component */ render() { - const { children, defaultText, width } = this.props; + const { children, defaultText, disabled, width } = this.props; const { open, focusedOption, selectedOption } = this.state; return ( section { padding: 0; } diff --git a/src/components/dropdown/dropdown.test.js b/src/components/dropdown/dropdown.test.js index e69de29bb2..78b815786e 100644 --- a/src/components/dropdown/dropdown.test.js +++ b/src/components/dropdown/dropdown.test.js @@ -0,0 +1,40 @@ +import React from 'react'; +import sinon from 'sinon'; +import { shallow } from 'enzyme'; +import Dropdown from './dropdown'; +import MenuOption from '../menu-option/menu-option'; + +const mockData = [ + { + defaultText: 'Test 123', + onOpened: sinon.spy(() => {}), + onClosed: sinon.spy(() => {}), + onChanged: sinon.spy(() => {}), + }, + { + defaultText: 'Test 456', + onOpened: sinon.spy(() => {}), + onClosed: sinon.spy(() => {}), + onChanged: sinon.spy(() => {}), + }, +]; + +mockData.forEach((dataSet, i) => { + const jsx = ( + + + + + + ); + describe(`Dropdown - Test ${i}`, () => { + it('should be a function', () => { + expect(typeof Dropdown).toBe('function'); + }); + + it('should create a valid React Component when called with required props', () => { + const wrapper = shallow(jsx); + expect(wrapper.children().length === 3).toBeTruthy(); + }); + }); +}); diff --git a/src/components/menu-option/menu-option.js b/src/components/menu-option/menu-option.js index a4719f400d..0f580e986b 100644 --- a/src/components/menu-option/menu-option.js +++ b/src/components/menu-option/menu-option.js @@ -15,9 +15,9 @@ const MenuOption = ({ selected, value, }) => { - const wrapperClasses = classnames('kedro', 'kui-menu-option', className, { - 'kui-menu-option--focused': focused, - 'kui-menu-option--selected': selected, + const wrapperClasses = classnames('kedro', 'menu-option', className, { + 'menu-option--focused': focused, + 'menu-option--selected': selected, }); /** @@ -53,7 +53,7 @@ const MenuOption = ({ role="option" tabIndex="-1" > -
+
{primaryText}
diff --git a/src/components/menu-option/menu-option.scss b/src/components/menu-option/menu-option.scss index fd06a0c48c..17f453dfa4 100644 --- a/src/components/menu-option/menu-option.scss +++ b/src/components/menu-option/menu-option.scss @@ -23,7 +23,7 @@ $menu-item-padding: 12px; --menu-option-bg-hover: #{rgba(white, 0.03)}; } -.kui-menu-option { +.menu-option { display: flex; align-items: center; box-sizing: border-box; @@ -40,14 +40,14 @@ $menu-item-padding: 12px; user-select: none; outline: none; - &.kui-menu-option--focused, + &.menu-option--focused, &:focus, &:hover { background: var(--menu-option-bg-hover); } } -.kui-menu-option__content { +.menu-option__content { position: relative; display: flex; align-items: center; diff --git a/src/components/menu-option/menu-option.test.js b/src/components/menu-option/menu-option.test.js index e69de29bb2..87aebf0ab5 100644 --- a/src/components/menu-option/menu-option.test.js +++ b/src/components/menu-option/menu-option.test.js @@ -0,0 +1,45 @@ +import React from 'react'; +import { setup } from '../../utils/state.mock'; +import sinon from 'sinon'; +import MenuOption from '../menu-option'; + +const mockData = [ + { + primaryText: 'Test 123', + onSelected: sinon.spy(() => {}), + }, + { + primaryText: 'Test 456', + onSelected: sinon.spy(() => {}), + }, +]; + +mockData.forEach((dataSet, i) => { + const jsx = ; + + describe(`Menu Option - Test ${i}`, () => { + it('should be a function', () => { + expect(typeof MenuOption).toBe('function'); + }); + + it('should contain text', () => { + const wrapper = setup.mount(jsx); + expect( + wrapper.find('.menu-option__content').text() === dataSet.primaryText + ).toBeTruthy(); + + expect( + wrapper.find(`.menu-option__content[title="${dataSet.primaryText}"]`) + .length === 1 + ).toBeTruthy(); + }); + + if (typeof dataSet.onSelected === 'function') { + it('should fire onSelected event handler when clicked', () => { + const wrapper = setup.mount(jsx); + wrapper.simulate('click'); + expect(dataSet.onSelected.called).toBeTruthy(); + }); + } + }); +}); diff --git a/src/components/pipeline-list/pipeline-list.scss b/src/components/pipeline-list/pipeline-list.scss index 9901c9e5c5..bc362e1036 100644 --- a/src/components/pipeline-list/pipeline-list.scss +++ b/src/components/pipeline-list/pipeline-list.scss @@ -14,12 +14,12 @@ } .pipeline-list { - .kui-dropdown__label, - .kui-menu-option { + .dropdown__label, + .menu-option { height: 58px; } - .kui-menu-option { + .menu-option { &.pipeline-list__option--active { border-left-color: $color-primary; } diff --git a/src/components/pipeline-list/pipeline-list.test.js b/src/components/pipeline-list/pipeline-list.test.js index 98c53139c9..0e93b65728 100644 --- a/src/components/pipeline-list/pipeline-list.test.js +++ b/src/components/pipeline-list/pipeline-list.test.js @@ -20,15 +20,15 @@ describe('PipelineList', () => { it('should call onToggleOpen when opening/closing', () => { const onToggleOpen = jest.fn(); const wrapper = setup.mount(); - wrapper.find('.kui-dropdown__label').simulate('click'); + wrapper.find('.dropdown__label').simulate('click'); expect(onToggleOpen).toHaveBeenLastCalledWith(true); - wrapper.find('.kui-dropdown__label').simulate('click'); + wrapper.find('.dropdown__label').simulate('click'); expect(onToggleOpen).toHaveBeenLastCalledWith(false); }); it('should be disabled when there are no pipelines in the store', () => { const wrapper = setup.mount(, { data: 'json' }); - expect(wrapper.find('.kui-dropdown__label').prop('disabled')).toBe(true); + expect(wrapper.find('.dropdown__label').prop('disabled')).toBe(true); }); test.each(pipelineIDs)( @@ -70,7 +70,6 @@ describe('PipelineList', () => { ids: expect.any(Array), }, prettyName: expect.any(Boolean), - theme: mockState.spaceflights.theme, }); }); From 71be293e46dc84af82b8b2141f61982ba7a81051 Mon Sep 17 00:00:00 2001 From: Rashida Kanchwala Date: Fri, 28 Jan 2022 09:45:41 +0000 Subject: [PATCH 05/18] Update release docs --- RELEASE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE.md b/RELEASE.md index f4d8a5f748..afb1be77c2 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -9,6 +9,7 @@ Please follow the established format: --> ## Bug fixes and other changes - Migrate Kedro-UI buttons to Kedro-viz as Kedro-UI is now deprecated. (#716) +- Migrate Kedro-UI dropdown and menu-options to Kedro-viz as Kedro-UI is now deprecated. (#716) # Release 4.3.1 From c273b92185698c0046fd490558f5a3b4344da4cc Mon Sep 17 00:00:00 2001 From: Rashida Kanchwala Date: Fri, 28 Jan 2022 09:48:36 +0000 Subject: [PATCH 06/18] Remove unwanted code Signed-off-by: Rashida Kanchwala --- src/styles/_extends.scss | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/src/styles/_extends.scss b/src/styles/_extends.scss index 7405d87968..e7264c93d0 100644 --- a/src/styles/_extends.scss +++ b/src/styles/_extends.scss @@ -68,28 +68,3 @@ font-size: 14px; font-weight: normal; } - -%shadow-light { - box-shadow: 0 0 2px 0 rgba(0, 0, 0, 0.24), 0 0 2px 0 rgba(0, 0, 0, 0.08); -} - -%shadow-light--hover { - box-shadow: -2px 0 4px 0 rgba(0, 0, 0, 0.08), 2px 0 4px 0 rgba(0, 0, 0, 0.08), - 0 2px 4px 0 rgba(0, 0, 0, 0.12), 0 0 2px 0 rgba(0, 0, 0, 0.12); -} - -%shadow-dark { - box-shadow: 0 0 2px 0 rgba(0, 0, 0, 0.24), 0 0 2px 0 rgba(0, 0, 0, 0.12), - inset 0 1px 1px 0 rgba(255, 255, 255, 0.03); -} - -%shadow-dark--hover { - box-shadow: -2px 0 4px 0 rgba(0, 0, 0, 0.08), 2px 0 4px 0 rgba(0, 0, 0, 0.08), - 0 2px 4px 0 rgba(0, 0, 0, 0.12), 0 0 2px 0 rgba(0, 0, 0, 0.24), - 0 0 2px 0 rgba(0, 0, 0, 0.24), 0 0 2px 0 rgba(0, 0, 0, 0.12), - inset 0 1px 1px 0 rgba(255, 255, 255, 0.03); -} - -%shadow-light--active { - box-shadow: 0 0 2px 0 rgba(0, 0, 0, 0.24), 0 0 2px 0 rgba(0, 0, 0, 0.08); -} From cebf30c9a19f0b6d93c1cbbb2879f787c8fe01aa Mon Sep 17 00:00:00 2001 From: Rashida Kanchwala Date: Tue, 8 Feb 2022 14:22:58 +0000 Subject: [PATCH 07/18] WIP - classes to hook --- src/components/dropdown/dropdown.js | 118 ++++++++++------------------ 1 file changed, 43 insertions(+), 75 deletions(-) diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js index a5c699a765..3f36ec7612 100644 --- a/src/components/dropdown/dropdown.js +++ b/src/components/dropdown/dropdown.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useEffect } from 'react'; import PropTypes from 'prop-types'; import { flatten, find, flow, isEqual, map } from 'lodash/fp'; import 'what-input'; @@ -6,62 +6,37 @@ import './dropdown.css'; import DropdownRenderer from './dropdown-renderer'; import EventController from './event-controller.js'; -class Dropdown extends React.Component { - /** - * Create a new Dropdown - * @param {Object} props - */ - constructor(props) { - super(props); - - this.displayName = 'Dropdown'; - - // bind method scope - this._handleRef = this._handleRef.bind(this); - this._getOptionsList = this._getOptionsList.bind(this); - this._handleLabelClicked = this._handleLabelClicked.bind(this); - this._handleOptionSelected = this._handleOptionSelected.bind(this); - this._handleFocusChange = this._handleFocusChange.bind(this); - this._handleBodyClicked = this._handleBodyClicked.bind(this); - this.open = this.open.bind(this); - this.close = this.close.bind(this); - - this.state = { - focusedOption: null, - selectedOption: this._findSelectedOption(), - open: false, - }; - } - - /** - * React lifecycle method - * {@link https://facebook.github.io/react/docs/react-component.html#componentwillupdate} - * @param {Object} New component props - */ - componentDidUpdate(prevProps) { - if (this._childrenHaveChanged(prevProps)) { - this.setState({ - selectedOption: this._findSelectedOption(prevProps), - }); - } - } - - /** - * React lifecycle method - * {@link https://facebook.github.io/react/docs/react-component.html#componentwillunmount} - * @return {object} JSX for this component - */ - componentWillUnmount() { - EventController.removeBodyListeners(); - } +const Dropdown = ({ + children, + defaultText, + disabled, + onChanged, + onClosed, + onOpened, + width, +})=>{ + + const [focusedOption, setFocusedOption] = useState(null); + const [selectedOption, setSelectedOption] = useState(findSelectedOption()); + const [open, setOpen] = useState(false); + + useEffect(() => { + setSelectedOption(findSelectedOption()) + }, [selectedOption]); + + useEffect(()=>{ + return()=> + EventController.removeBodyListeners(); + }); + const dropdownRef= useRef(null) /** * Handler for closing a dropdown if a click occured outside the dropdown. * @param {object} e - event object */ - _handleBodyClicked(e) { - if (!this.dropdown.contains(e.target) && this.state.open) { - this.close(); + const _handleBodyClicked = (e) => { + if (!dropdownRef.current.contains(e.target) && open) { + setOpen(false) } } @@ -70,7 +45,7 @@ class Dropdown extends React.Component { * @param {Object} nextProps - New component props * @return {Boolean} True if new children are different from current ones */ - _childrenHaveChanged(nextProps) { + const _childrenHaveChanged = (nextProps) => { const children = [this.props, nextProps].map((props) => React.Children.toArray(props.children) ); @@ -83,7 +58,7 @@ class Dropdown extends React.Component { * @param {Object} props - Component props * @return {Object} Selected option object for use in the state */ - _findSelectedOption(props) { + const _findSelectedOption = (props) => { const selectedOptionElement = this._findSelectedOptionElement(props); // check children for a selected option @@ -110,7 +85,7 @@ class Dropdown extends React.Component { * @param {Object} props - Component props (optional) * @return {Object} Selected option element */ - _findSelectedOptionElement(props = this.props) { + const _findSelectedOptionElement = (props = this.props) => { const children = React.Children.toArray(props.children); if (!children.length) { @@ -133,7 +108,7 @@ class Dropdown extends React.Component { /** * Event handler which is fired when the label is clicked */ - _handleLabelClicked() { + const _handleLabelClicked = () => { const { open } = this.state; const { onOpened, onClosed } = this.props; @@ -162,7 +137,7 @@ class Dropdown extends React.Component { * with any Sections removed. * @return {Object} A flat list of MenuOptions */ - _getOptionsList() { + const _getOptionsList = () => { /** * Recurse through sections to retrieve a list of all MenuOptions * @param {Object} previous The Options array as of the previous iteration @@ -193,7 +168,7 @@ class Dropdown extends React.Component { * Convenience method to return focus from an option to the label. * This is particularly useful for screen-readers and keyboard users. */ - _focusLabel() { + const _focusLabel = () => { this.dropdown.querySelector('.dropdown__label').focus(); this.setState({ @@ -207,7 +182,7 @@ class Dropdown extends React.Component { * @param {number} direction - The direction that focus is travelling through the list: * negative is up and positive is down. */ - _handleFocusChange(direction) { + const _handleFocusChange = (direction) => { let { focusedOption } = this.state; const optionsLength = this._getOptionsList().length; @@ -233,7 +208,7 @@ class Dropdown extends React.Component { /** * Event handler which is fired when a child item is selected */ - _handleOptionSelected(obj) { + const _handleOptionSelected = (obj) => { const { label, id, value } = obj; const { onChanged, onClosed } = this.props; @@ -265,16 +240,16 @@ class Dropdown extends React.Component { * and assign it to a class-wide variable property. * @param {object} el - The ref for the Dropdown container node */ - _handleRef(el) { + const _handleRef = (el) => { this.dropdown = el; } /** * API method to open the dropdown */ - open() { - const { onOpened } = this.props; - + const _handleOpen = () => { + + setOpen(true) this.setState({ open: true }, () => { this._focusLabel(); if (typeof onOpened === 'function') { @@ -289,8 +264,7 @@ class Dropdown extends React.Component { /** * API method to close the dropdown */ - close() { - const { onClosed } = this.props; + const _handleClose = () => { this.setState({ open: false }, () => { if (typeof onClosed === 'function') { @@ -302,14 +276,7 @@ class Dropdown extends React.Component { EventController.removeBodyListeners(); } - /** - * React lifecycle method - * {@link https://facebook.github.io/react/docs/react-component.html#render} - * @return {object} JSX for this component - */ - render() { - const { children, defaultText, disabled, width } = this.props; - const { open, focusedOption, selectedOption } = this.state; + return ( {children} ); - } + } Dropdown.defaultProps = { From ddc466445dc4582e909a847df4ade3aae49c230f Mon Sep 17 00:00:00 2001 From: studioswong Date: Thu, 10 Feb 2022 10:05:47 +0000 Subject: [PATCH 08/18] refactored dropdown.js to use hooks --- src/components/app/app.test.js | 4 +- src/components/dropdown/dropdown.js | 317 +++++++++++++++------------- src/utils/hooks.js | 13 ++ 3 files changed, 185 insertions(+), 149 deletions(-) create mode 100644 src/utils/hooks.js diff --git a/src/components/app/app.test.js b/src/components/app/app.test.js index 2470ddfc21..8e3f309723 100644 --- a/src/components/app/app.test.js +++ b/src/components/app/app.test.js @@ -97,10 +97,12 @@ describe('App', () => { const pipelineDropdownLabel = pipelineDropdown.querySelector( '.dropdown__label > span:first-child' ); + expect(pipelineDropdownLabel.innerHTML).toBe('Default'); fireEvent.click(menuOption); expect(pipelineDropdownLabel.innerHTML).toBe(activePipeline.name); rerender(); - expect(pipelineDropdownLabel.innerHTML).toBe('Default'); + // the default dropdown placeholder is 'Please select...' which is not returned right after a rerender + expect(pipelineDropdownLabel.innerHTML).toBe('Please select...'); }); }); diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js index 3f36ec7612..1bc122e8e7 100644 --- a/src/components/dropdown/dropdown.js +++ b/src/components/dropdown/dropdown.js @@ -1,57 +1,22 @@ -import React, { useEffect } from 'react'; +import React, { useEffect, useState, useRef } from 'react'; import PropTypes from 'prop-types'; import { flatten, find, flow, isEqual, map } from 'lodash/fp'; import 'what-input'; import './dropdown.css'; import DropdownRenderer from './dropdown-renderer'; import EventController from './event-controller.js'; - -const Dropdown = ({ - children, - defaultText, - disabled, - onChanged, - onClosed, - onOpened, - width, -})=>{ - - const [focusedOption, setFocusedOption] = useState(null); - const [selectedOption, setSelectedOption] = useState(findSelectedOption()); - const [open, setOpen] = useState(false); - - useEffect(() => { - setSelectedOption(findSelectedOption()) - }, [selectedOption]); - - useEffect(()=>{ - return()=> - EventController.removeBodyListeners(); - }); - const dropdownRef= useRef(null) - - /** - * Handler for closing a dropdown if a click occured outside the dropdown. - * @param {object} e - event object - */ - const _handleBodyClicked = (e) => { - if (!dropdownRef.current.contains(e.target) && open) { - setOpen(false) - } - } - - /** - * Check whether new props contain updated children - * @param {Object} nextProps - New component props - * @return {Boolean} True if new children are different from current ones - */ - const _childrenHaveChanged = (nextProps) => { - const children = [this.props, nextProps].map((props) => - React.Children.toArray(props.children) - ); - - return !isEqual(...children); - } +import { usePrevious } from '../../utils/hooks'; + +const Dropdown = (props) => { + const { + children, + defaultText, + disabled, + onChanged, + onClosed, + onOpened, + width, + } = props; /** * Format the selected option props for adding to state @@ -59,7 +24,7 @@ const Dropdown = ({ * @return {Object} Selected option object for use in the state */ const _findSelectedOption = (props) => { - const selectedOptionElement = this._findSelectedOptionElement(props); + const selectedOptionElement = _findSelectedOptionElement(props); // check children for a selected option if (selectedOptionElement) { @@ -78,14 +43,14 @@ const Dropdown = ({ label: null, value: null, }; - } + }; /** * Find the selected option by traversing sections and MenuOptions * @param {Object} props - Component props (optional) * @return {Object} Selected option element */ - const _findSelectedOptionElement = (props = this.props) => { + const _findSelectedOptionElement = (props) => { const children = React.Children.toArray(props.children); if (!children.length) { @@ -103,34 +68,125 @@ const Dropdown = ({ } return find((child) => child.props.selected)(children); - } + }; + + const prevProps = usePrevious(props); + const [focusedOption, setFocusedOption] = useState(null); + const [haveClicked, setHaveClicked] = useState(false); // tracker for detecting _handleLabelClicked + const [selectedOption, setSelectedOption] = useState( + _findSelectedOption(props) + ); + const [open, setOpen] = useState(false); + const [selectedObject, setSelectedObject] = useState(null); // this is to store the object that was passed from the handleOptionSelected eventhandler + + const dropdownRef = useRef(); + const handleOptionSelectedRef = useRef({ open, selectedOption }); + const selectedObjRef = useRef(selectedObject); + const openRef = useRef(open); + + const mounted = useRef(false); // ref for detecting mounting of component + + useEffect(() => { + if (!mounted.current) { + // update mounted on componentDidMount + mounted.current = true; + } else { + // triggers every time on componentDidUpdate + if (_childrenHaveChanged(prevProps)) { + setSelectedOption(_findSelectedOption(prevProps)); + } + } + }); + + useEffect(() => { + if (haveClicked === true) { + console.log('have clicked'); + console.log('open', open); + // set callbacks, if defined + if (typeof onOpened === 'function' && open) { + onOpened(); + } else if (typeof onClosed === 'function' && !open) { + onClosed(); + } + + // reset haveClicked + setHaveClicked(false); + } + }, [haveClicked, onOpened, onClosed, open]); + + // use effect to be fired after state changes triggered by handleOptionSelected eventhandler + useEffect(() => { + // this check is to ensure that only the changes in the handleOptionSelected eventhandler will trigger this useEffect + if (selectedObjRef.current !== selectedObject) { + if ( + !open && + handleOptionSelectedRef.current.handleOptionSelectedRef !== + selectedOption + ) { + if (typeof onChanged === 'function') { + onChanged(selectedObject); + } + } + if (!open && typeof onClosed === 'function') { + onClosed(); + } + } + }); + + // event to be fired on componentWillUnmount + useEffect(() => { + return () => EventController.removeBodyListeners(); + }, []); + + useEffect(() => { + // Focus either the button label or the active option. + // This is so screen-readers will follow the active element + const focusClass = + focusedOption !== null ? '.menu-option--focused' : '.dropdown__label'; + + dropdownRef.current.querySelector(focusClass).focus(); + }, [focusedOption]); /** - * Event handler which is fired when the label is clicked + * Handler for closing a dropdown if a click occured outside the dropdown. + * @param {object} e - event object */ - const _handleLabelClicked = () => { - const { open } = this.state; - const { onOpened, onClosed } = this.props; + const _handleBodyClicked = (e) => { + if (!dropdownRef.current.contains(e.target) && open) { + _handleClose(); + } + }; - let callback = null; + /** + * Check whether new props contain updated children + * @param {Object} nextProps - New component props + * @return {Boolean} True if new children are different from current ones + */ + const _childrenHaveChanged = (nextProps) => { + const children = [props, nextProps].map((props) => + React.Children.toArray(props.children) + ); - // set callbacks, if defined - if (typeof onOpened === 'function' && !open) { - callback = onOpened; - } else if (typeof onClosed === 'function' && open) { - callback = onClosed; - } + return !isEqual(...children); + }; + /** + * Event handler which is fired when the label is clicked + */ + const _handleLabelClicked = () => { // remove or add the event listeners for if (open) { EventController.removeBodyListeners(); } else { - EventController.addBodyListener(this._handleBodyClicked); + EventController.addBodyListener(_handleBodyClicked); } + // revert the state of open + setOpen(!open); + // set the click tracker to true to trigger the userEffect callback + setHaveClicked(true); - this.setState({ open: !open }, callback); - this._focusLabel(); - } + _focusLabel(); + }; /** * Sort, filter and flatten the list of children to retrieve just the MenuOptions, @@ -162,19 +218,17 @@ const Dropdown = ({ getSectionChildren, [] ); - } + }; /** * Convenience method to return focus from an option to the label. * This is particularly useful for screen-readers and keyboard users. */ const _focusLabel = () => { - this.dropdown.querySelector('.dropdown__label').focus(); + dropdownRef.current.querySelector('.dropdown__label').focus(); - this.setState({ - focusedOption: null, - }); - } + setFocusedOption(null); + }; /** * When the focused option changes (e.g. via up/down keyboard controls), @@ -183,57 +237,43 @@ const Dropdown = ({ * negative is up and positive is down. */ const _handleFocusChange = (direction) => { - let { focusedOption } = this.state; - const optionsLength = this._getOptionsList().length; + let newFocusedOption = focusedOption; + const optionsLength = _getOptionsList().length; if (focusedOption === null) { - focusedOption = direction > 0 ? 0 : optionsLength - 1; + newFocusedOption = direction > 0 ? 0 : optionsLength - 1; } else { - focusedOption += direction; + newFocusedOption += direction; } - if (focusedOption >= optionsLength || focusedOption < 0) { - focusedOption = null; + if (newFocusedOption >= optionsLength || newFocusedOption < 0) { + newFocusedOption = null; } - this.setState({ focusedOption }, () => { - // Focus either the button label or the active option. - // This is so screen-readers will follow the active element - const focusClass = - focusedOption !== null ? '.menu-option--focused' : '.dropdown__label'; - - this.dropdown.querySelector(focusClass).focus(); - }); - } + setFocusedOption(newFocusedOption); + }; /** * Event handler which is fired when a child item is selected */ const _handleOptionSelected = (obj) => { const { label, id, value } = obj; - const { onChanged, onClosed } = this.props; + // this is not needed now as it is imported + // const { onChanged, onClosed } = this.props; + + setSelectedObject(obj); // detect if the selected item has changed - const hasChanged = value !== this.state.selectedOption.value; + const hasChanged = value !== selectedOption.value; if (hasChanged) { - const selectedOption = { label, value, id }; - this.setState({ open: false, selectedOption }, () => { - if (typeof onChanged === 'function') { - onChanged(obj); - } + const newSelectedOption = { label, value, id }; - if (typeof onClosed === 'function') { - onClosed(); - } - }); + setOpen(false); + setSelectedOption(newSelectedOption); } else { - this.setState({ open: false }, () => { - if (typeof onClosed === 'function') { - onClosed(); - } - }); + setOpen(false); } - this._focusLabel(); - } + _focusLabel(); + }; /** * Retrieve a reference to the dropdown DOM node (from the renderer component), @@ -241,30 +281,14 @@ const Dropdown = ({ * @param {object} el - The ref for the Dropdown container node */ const _handleRef = (el) => { - this.dropdown = el; - } - - /** - * API method to open the dropdown - */ - const _handleOpen = () => { - - setOpen(true) - this.setState({ open: true }, () => { - this._focusLabel(); - if (typeof onOpened === 'function') { - onOpened(); - } - }); - - // add event listener to automatically close the dropdown - EventController.addBodyListener(this._handleBodyClicked); - } + dropdownRef.current = el; + }; /** * API method to close the dropdown */ const _handleClose = () => { + setOpen(false); this.setState({ open: false }, () => { if (typeof onClosed === 'function') { @@ -274,29 +298,26 @@ const Dropdown = ({ // remove event listener EventController.removeBodyListeners(); - } - - - - return ( - - {children} - - ); - -} + }; + + return ( + + {children} + + ); +}; Dropdown.defaultProps = { children: null, diff --git a/src/utils/hooks.js b/src/utils/hooks.js new file mode 100644 index 0000000000..34e3e3f0f0 --- /dev/null +++ b/src/utils/hooks.js @@ -0,0 +1,13 @@ +import { useEffect, useRef } from 'react'; + +/** + * custom hook to obtain previous values before state changes, value can be any data type + * @param {value} object + */ +export const usePrevious = (value) => { + const ref = useRef(); + useEffect(() => { + ref.current = value; + }); + return ref.current; +}; From 5471eab051512ddf3e174b8ae1856288e8683231 Mon Sep 17 00:00:00 2001 From: studioswong Date: Thu, 10 Feb 2022 14:49:08 +0000 Subject: [PATCH 09/18] removed unneccessary useRef --- src/components/dropdown/dropdown.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js index 1bc122e8e7..3db441f058 100644 --- a/src/components/dropdown/dropdown.js +++ b/src/components/dropdown/dropdown.js @@ -82,7 +82,6 @@ const Dropdown = (props) => { const dropdownRef = useRef(); const handleOptionSelectedRef = useRef({ open, selectedOption }); const selectedObjRef = useRef(selectedObject); - const openRef = useRef(open); const mounted = useRef(false); // ref for detecting mounting of component From 38cbd367169b4c87cb4d8ae272c0c53827fc3006 Mon Sep 17 00:00:00 2001 From: Rashida Kanchwala Date: Thu, 10 Feb 2022 15:55:00 +0000 Subject: [PATCH 10/18] Added up/down arrow --- src/components/dropdown/dropdown-renderer.js | 7 +++++++ src/components/dropdown/dropdown.scss | 15 +++++++++++++-- src/components/icons/dropdown-arrow.js | 9 +++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 src/components/icons/dropdown-arrow.js diff --git a/src/components/dropdown/dropdown-renderer.js b/src/components/dropdown/dropdown-renderer.js index 9cfc73b6b9..040500d4f2 100644 --- a/src/components/dropdown/dropdown-renderer.js +++ b/src/components/dropdown/dropdown-renderer.js @@ -3,6 +3,8 @@ import PropTypes from 'prop-types'; import classnames from 'classnames'; import handleKeyEvent from '../../utils/key-events'; import uniqueId from 'lodash/uniqueId'; +import DropdownArrow from '../icons/dropdown-arrow'; +import IconButton from '../icon-button'; /** * Renderer for the Dropdown component @@ -127,6 +129,11 @@ const DropdownRenderer = ({ onClick={onLabelClicked} > {selectedOption.label || defaultText} +
{optionsNode}
diff --git a/src/components/dropdown/dropdown.scss b/src/components/dropdown/dropdown.scss index 1658d74e45..12ef7fddfd 100644 --- a/src/components/dropdown/dropdown.scss +++ b/src/components/dropdown/dropdown.scss @@ -43,17 +43,23 @@ $menu-item-padding: 12px; position: relative; z-index: 9; + + .dropdown__icon { + transform: rotate(180deg); + transition: transform ease 0.2s, background ease 0.2s; + } } .dropdown__label { position: relative; z-index: 8; - + list-style: none; display: flex; + justify-content: space-between; width: 100%; align-items: center; height: $menu-item-height; - padding: 0 $sidebar-padding / 1.6; // divide by 1.6em font-size on element + padding: 0 0 0 $sidebar-padding / 1.6; // divide by 1.6em font-size on element overflow: hidden; border: none; border-radius: 0; @@ -82,6 +88,11 @@ $menu-item-padding: 12px; } } } + + .dropdown__icon { + float: right; + transition: transform ease 0.2s, background ease 0.2s; + } } .dropdown__options { diff --git a/src/components/icons/dropdown-arrow.js b/src/components/icons/dropdown-arrow.js new file mode 100644 index 0000000000..df04bfa10f --- /dev/null +++ b/src/components/icons/dropdown-arrow.js @@ -0,0 +1,9 @@ +import React from 'react'; + +const DropdownArrow = ({ className }) => ( + + + +); + +export default DropdownArrow; From 8bdf9e6cc814f1a886c75b9de069eeb24ad35ab6 Mon Sep 17 00:00:00 2001 From: Tynan DeBold Date: Thu, 10 Feb 2022 17:59:38 +0000 Subject: [PATCH 11/18] style alignments Signed-off-by: Tynan DeBold --- package-lock.json | 2 +- package.json | 2 +- src/components/dropdown/dropdown-renderer.js | 9 ++-- src/components/dropdown/dropdown.js | 44 ++++++++------- src/components/dropdown/dropdown.scss | 56 ++++++++++++-------- src/components/menu-option/menu-option.scss | 15 ++---- src/utils/hooks.js | 2 +- 7 files changed, 65 insertions(+), 65 deletions(-) diff --git a/package-lock.json b/package-lock.json index d740fe83de..dc022f5115 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24817,4 +24817,4 @@ "dev": true } } -} \ No newline at end of file +} diff --git a/package.json b/package.json index 2cad5a31e9..d6bc16fd6e 100644 --- a/package.json +++ b/package.json @@ -138,4 +138,4 @@ "not op_mini all" ], "snyk": true -} \ No newline at end of file +} diff --git a/src/components/dropdown/dropdown-renderer.js b/src/components/dropdown/dropdown-renderer.js index 040500d4f2..3547d7cf10 100644 --- a/src/components/dropdown/dropdown-renderer.js +++ b/src/components/dropdown/dropdown-renderer.js @@ -4,7 +4,6 @@ import classnames from 'classnames'; import handleKeyEvent from '../../utils/key-events'; import uniqueId from 'lodash/uniqueId'; import DropdownArrow from '../icons/dropdown-arrow'; -import IconButton from '../icon-button'; /** * Renderer for the Dropdown component @@ -129,11 +128,9 @@ const DropdownRenderer = ({ onClick={onLabelClicked} > {selectedOption.label || defaultText} - + + +
{optionsNode}
diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js index 3db441f058..915b70bafd 100644 --- a/src/components/dropdown/dropdown.js +++ b/src/components/dropdown/dropdown.js @@ -1,4 +1,4 @@ -import React, { useEffect, useState, useRef } from 'react'; +import React, { useCallback, useEffect, useState, useRef } from 'react'; import PropTypes from 'prop-types'; import { flatten, find, flow, isEqual, map } from 'lodash/fp'; import 'what-input'; @@ -23,7 +23,7 @@ const Dropdown = (props) => { * @param {Object} props - Component props * @return {Object} Selected option object for use in the state */ - const _findSelectedOption = (props) => { + const _findSelectedOption = useCallback((props) => { const selectedOptionElement = _findSelectedOptionElement(props); // check children for a selected option @@ -43,7 +43,7 @@ const Dropdown = (props) => { label: null, value: null, }; - }; + }, []); /** * Find the selected option by traversing sections and MenuOptions @@ -77,7 +77,7 @@ const Dropdown = (props) => { _findSelectedOption(props) ); const [open, setOpen] = useState(false); - const [selectedObject, setSelectedObject] = useState(null); // this is to store the object that was passed from the handleOptionSelected eventhandler + const [selectedObject, setSelectedObject] = useState(null); // this is to store the object that was passed from the handleOptionSelected event handler const dropdownRef = useRef(); const handleOptionSelectedRef = useRef({ open, selectedOption }); @@ -86,6 +86,19 @@ const Dropdown = (props) => { const mounted = useRef(false); // ref for detecting mounting of component useEffect(() => { + /** + * Check whether new props contain updated children + * @param {Object} nextProps - New component props + * @return {Boolean} True if new children are different from current ones + */ + const _childrenHaveChanged = (nextProps) => { + const children = [props, nextProps].map((props) => + React.Children.toArray(props.children) + ); + + return !isEqual(...children); + }; + if (!mounted.current) { // update mounted on componentDidMount mounted.current = true; @@ -95,12 +108,10 @@ const Dropdown = (props) => { setSelectedOption(_findSelectedOption(prevProps)); } } - }); + }, [_findSelectedOption, prevProps, props]); useEffect(() => { if (haveClicked === true) { - console.log('have clicked'); - console.log('open', open); // set callbacks, if defined if (typeof onOpened === 'function' && open) { onOpened(); @@ -113,9 +124,9 @@ const Dropdown = (props) => { } }, [haveClicked, onOpened, onClosed, open]); - // use effect to be fired after state changes triggered by handleOptionSelected eventhandler + // Use effect to be fired after state changes triggered by handleOptionSelected event handler useEffect(() => { - // this check is to ensure that only the changes in the handleOptionSelected eventhandler will trigger this useEffect + // This check is to ensure that only the changes in the handleOptionSelected event handler will trigger this useEffect if (selectedObjRef.current !== selectedObject) { if ( !open && @@ -132,7 +143,7 @@ const Dropdown = (props) => { } }); - // event to be fired on componentWillUnmount + // Event to be fired on componentWillUnmount useEffect(() => { return () => EventController.removeBodyListeners(); }, []); @@ -156,19 +167,6 @@ const Dropdown = (props) => { } }; - /** - * Check whether new props contain updated children - * @param {Object} nextProps - New component props - * @return {Boolean} True if new children are different from current ones - */ - const _childrenHaveChanged = (nextProps) => { - const children = [props, nextProps].map((props) => - React.Children.toArray(props.children) - ); - - return !isEqual(...children); - }; - /** * Event handler which is fired when the label is clicked */ diff --git a/src/components/dropdown/dropdown.scss b/src/components/dropdown/dropdown.scss index 12ef7fddfd..85c0f82c4e 100644 --- a/src/components/dropdown/dropdown.scss +++ b/src/components/dropdown/dropdown.scss @@ -30,49 +30,46 @@ $menu-item-padding: 12px; } .dropdown { - position: relative; - box-sizing: border-box; - display: block; border: none; box-shadow: 0 1px 1px var(--dropdown-shadow); + box-sizing: border-box; color: var(--color-text); - border: 1px solid var(--dropdown-border); + display: block; + position: relative; } .dropdown--open { position: relative; - z-index: 9; - - .dropdown__icon { - transform: rotate(180deg); + .dropdown__label > .dropdown__icon { + transform: rotate(0deg); transition: transform ease 0.2s, background ease 0.2s; } } .dropdown__label { - position: relative; - z-index: 8; - list-style: none; - display: flex; - justify-content: space-between; - width: 100%; align-items: center; - height: $menu-item-height; - padding: 0 0 0 $sidebar-padding / 1.6; // divide by 1.6em font-size on element - overflow: hidden; - border: none; + background: var(--dropdown-label-bg); border-radius: 0; + border: none; box-shadow: none; - outline: 4px solid transparent; + color: var(--color-text); + cursor: pointer; + display: flex; font-family: inherit; font-size: 1.6em; + height: $menu-item-height; + justify-content: space-between; letter-spacing: 0.1px; line-height: 1.25em; - background: var(--dropdown-label-bg); - color: var(--color-text); - cursor: pointer; + list-style: none; + outline: 4px solid transparent; + overflow: hidden; + padding: 0 0 0 $sidebar-padding / 1.6; // divide by 1.6em font-size on element + position: relative; user-select: none; + width: 100%; + z-index: 8; &:disabled { cursor: not-allowed; @@ -90,8 +87,21 @@ $menu-item-padding: 12px; } .dropdown__icon { - float: right; + height: 24px; + margin: 0 8px 0 12px; + position: absolute; + right: 0; + transform: rotate(180deg); + transform-origin: center; transition: transform ease 0.2s, background ease 0.2s; + width: auto; + + > svg { + fill: var(--color-default-alt); + height: 24px; + opacity: 0.5; + width: 24px; + } } } diff --git a/src/components/menu-option/menu-option.scss b/src/components/menu-option/menu-option.scss index 17f453dfa4..4ee39ce482 100644 --- a/src/components/menu-option/menu-option.scss +++ b/src/components/menu-option/menu-option.scss @@ -24,21 +24,16 @@ $menu-item-padding: 12px; } .menu-option { - display: flex; align-items: center; box-sizing: border-box; - + cursor: pointer; + display: flex; height: $menu-item-height; - - padding: 0 $sidebar-padding 0 calc(#{$sidebar-padding} - 2px); - border-color: var(--menu-option-border-color) transparent; - border-style: solid; - border-width: var(--menu-option-border-width); - white-space: nowrap; + outline: none; overflow: hidden; - cursor: pointer; + padding: 0 $sidebar-padding 0 calc(#{$sidebar-padding} - 2px); user-select: none; - outline: none; + white-space: nowrap; &.menu-option--focused, &:focus, diff --git a/src/utils/hooks.js b/src/utils/hooks.js index 34e3e3f0f0..d03674f284 100644 --- a/src/utils/hooks.js +++ b/src/utils/hooks.js @@ -1,7 +1,7 @@ import { useEffect, useRef } from 'react'; /** - * custom hook to obtain previous values before state changes, value can be any data type + * Custom hook to obtain previous values before state changes. The value can be any data type. * @param {value} object */ export const usePrevious = (value) => { From 24ff11a016cd05daa740a2573f34b6fcd4443a78 Mon Sep 17 00:00:00 2001 From: Tynan DeBold Date: Thu, 10 Feb 2022 18:10:36 +0000 Subject: [PATCH 12/18] fix console error Signed-off-by: Tynan DeBold --- src/components/dropdown/dropdown.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js index 915b70bafd..49b86f4e62 100644 --- a/src/components/dropdown/dropdown.js +++ b/src/components/dropdown/dropdown.js @@ -309,7 +309,6 @@ const Dropdown = (props) => { focusedOption={focusedOption} selectedOption={selectedOption} width={width} - ref={dropdownRef} > {children} From 525fb7154c6ff74a23dac837dec068d75b33c397 Mon Sep 17 00:00:00 2001 From: Tynan DeBold Date: Thu, 10 Feb 2022 18:11:50 +0000 Subject: [PATCH 13/18] cleanup Signed-off-by: Tynan DeBold --- src/components/dropdown/dropdown.js | 7 ++++--- src/components/dropdown/event-controller.js | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js index 49b86f4e62..7d9ca7f020 100644 --- a/src/components/dropdown/dropdown.js +++ b/src/components/dropdown/dropdown.js @@ -1,11 +1,12 @@ import React, { useCallback, useEffect, useState, useRef } from 'react'; import PropTypes from 'prop-types'; -import { flatten, find, flow, isEqual, map } from 'lodash/fp'; import 'what-input'; -import './dropdown.css'; -import DropdownRenderer from './dropdown-renderer'; +import { flatten, find, flow, isEqual, map } from 'lodash/fp'; import EventController from './event-controller.js'; import { usePrevious } from '../../utils/hooks'; +import DropdownRenderer from './dropdown-renderer'; + +import './dropdown.css'; const Dropdown = (props) => { const { diff --git a/src/components/dropdown/event-controller.js b/src/components/dropdown/event-controller.js index ea6ab4b0df..c5fbc8539a 100644 --- a/src/components/dropdown/event-controller.js +++ b/src/components/dropdown/event-controller.js @@ -12,7 +12,7 @@ class EventController { window.__bodyEventHandlers = []; } - // add event handler to the array attached to the windown so that it can be retrieved outside of component + // add event handler to the array attached to the window so that it can be retrieved outside of component window.__bodyEventHandlers.push(eventHandler); // add the event handler to the body document.body.addEventListener('click', eventHandler); From 76a90feb047c3ebde6067d6b833d7e12bfea579d Mon Sep 17 00:00:00 2001 From: Tynan DeBold Date: Thu, 10 Feb 2022 18:29:09 +0000 Subject: [PATCH 14/18] keyboard events Signed-off-by: Tynan DeBold --- src/components/dropdown/dropdown.js | 12 +++++------- src/utils/key-events.js | 1 + 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js index 7d9ca7f020..ff13c279e5 100644 --- a/src/components/dropdown/dropdown.js +++ b/src/components/dropdown/dropdown.js @@ -159,7 +159,7 @@ const Dropdown = (props) => { }, [focusedOption]); /** - * Handler for closing a dropdown if a click occured outside the dropdown. + * Handler for closing a dropdown if a click occurred outside the dropdown. * @param {object} e - event object */ const _handleBodyClicked = (e) => { @@ -212,7 +212,7 @@ const Dropdown = (props) => { return previous; }; - return React.Children.toArray(this.props.children).reduce( + return React.Children.toArray(props.children).reduce( getSectionChildren, [] ); @@ -288,11 +288,9 @@ const Dropdown = (props) => { const _handleClose = () => { setOpen(false); - this.setState({ open: false }, () => { - if (typeof onClosed === 'function') { - onClosed(); - } - }); + if (typeof onClosed === 'function') { + onClosed(); + } // remove event listener EventController.removeBodyListeners(); diff --git a/src/utils/key-events.js b/src/utils/key-events.js index 6c46758ffd..79820a09d0 100644 --- a/src/utils/key-events.js +++ b/src/utils/key-events.js @@ -1,5 +1,6 @@ // Keyboard character codes const KEYS = { + 13: 'enter', 27: 'escape', 38: 'up', 40: 'down', From 19036162e5e83a9c23a5e82b2d49bcb9fda7f559 Mon Sep 17 00:00:00 2001 From: Tynan DeBold Date: Thu, 10 Feb 2022 18:33:09 +0000 Subject: [PATCH 15/18] update release notes Signed-off-by: Tynan DeBold --- RELEASE.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index b475a2df5a..454267d461 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -8,8 +8,8 @@ Please follow the established format: - Include the ID number for the related PR (or PRs) in parentheses --> ## Bug fixes and other changes -- Migrate Kedro-UI buttons to Kedro-viz as Kedro-UI is now deprecated. (#716) -- Migrate Kedro-UI dropdown and menu-options to Kedro-viz as Kedro-UI is now deprecated. (#716) +- Migrate Kedro-UI buttons to Kedro-Viz as Kedro-UI is now deprecated. (#716) +- Migrate Kedro-UI dropdown and menu-options to Kedro-Viz as Kedro-UI is now deprecated. (#716) - Add a Husky pre-push hook. (#723) - Create a `version` GraphQL query to get versions of Kedro-Viz. (#727) - Fix Kedro-Viz to work with projects that have no `__default__` registered pipeline. This also fixes the `--pipeline` CLI option. (#729) @@ -122,8 +122,8 @@ Please follow the established format: - Overwrite material UI selected row defaults. (#568) - Fix URI param parsing for data source. (#578) -- Add a graphql test endpoint on Kedro-viz server. (#570) -- Update the demo dataset on Kedro-viz. (#574) +- Add a graphql test endpoint on Kedro-Viz server. (#570) +- Update the demo dataset on Kedro-Viz. (#574) - Fix auto-reload for metrics run data. (#572) - Refactor tests for metadata panel. (#580) - Fix metrics tree to get latest metrics data. (#573) From 883d76c355a5d4ba94c11884ca44d5076592065f Mon Sep 17 00:00:00 2001 From: Tynan DeBold Date: Thu, 10 Feb 2022 18:33:39 +0000 Subject: [PATCH 16/18] update release notes again Signed-off-by: Tynan DeBold --- RELEASE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE.md b/RELEASE.md index 454267d461..33c68676b0 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -9,7 +9,7 @@ Please follow the established format: --> ## Bug fixes and other changes - Migrate Kedro-UI buttons to Kedro-Viz as Kedro-UI is now deprecated. (#716) -- Migrate Kedro-UI dropdown and menu-options to Kedro-Viz as Kedro-UI is now deprecated. (#716) +- Migrate Kedro-UI dropdown and menu-options to Kedro-Viz as Kedro-UI is now deprecated. (#721) - Add a Husky pre-push hook. (#723) - Create a `version` GraphQL query to get versions of Kedro-Viz. (#727) - Fix Kedro-Viz to work with projects that have no `__default__` registered pipeline. This also fixes the `--pipeline` CLI option. (#729) From 4097d2d7b9f89c08edc750e59d66312bb53aa8f5 Mon Sep 17 00:00:00 2001 From: studioswong Date: Tue, 15 Feb 2022 09:50:36 +0000 Subject: [PATCH 17/18] further cleanup of comments --- src/components/dropdown/dropdown.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js index ff13c279e5..1182986fb2 100644 --- a/src/components/dropdown/dropdown.js +++ b/src/components/dropdown/dropdown.js @@ -113,21 +113,19 @@ const Dropdown = (props) => { useEffect(() => { if (haveClicked === true) { - // set callbacks, if defined if (typeof onOpened === 'function' && open) { onOpened(); } else if (typeof onClosed === 'function' && !open) { onClosed(); } - // reset haveClicked setHaveClicked(false); } }, [haveClicked, onOpened, onClosed, open]); - // Use effect to be fired after state changes triggered by handleOptionSelected event handler + // to be fired after state changes triggered by handleOptionSelected event handler useEffect(() => { - // This check is to ensure that only the changes in the handleOptionSelected event handler will trigger this useEffect + // This check is to ensure that only the changes in the handleOptionSelected event handler will trigger this effect if (selectedObjRef.current !== selectedObject) { if ( !open && @@ -255,8 +253,6 @@ const Dropdown = (props) => { */ const _handleOptionSelected = (obj) => { const { label, id, value } = obj; - // this is not needed now as it is imported - // const { onChanged, onClosed } = this.props; setSelectedObject(obj); From 8ad351106850af5824a4ed9a60d93111abfe31b5 Mon Sep 17 00:00:00 2001 From: studioswong Date: Tue, 15 Feb 2022 10:04:07 +0000 Subject: [PATCH 18/18] removed unneeded function --- src/components/dropdown/dropdown.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/components/dropdown/dropdown.js b/src/components/dropdown/dropdown.js index 1182986fb2..1bf437eca1 100644 --- a/src/components/dropdown/dropdown.js +++ b/src/components/dropdown/dropdown.js @@ -284,10 +284,6 @@ const Dropdown = (props) => { const _handleClose = () => { setOpen(false); - if (typeof onClosed === 'function') { - onClosed(); - } - // remove event listener EventController.removeBodyListeners(); };