Skip to content

Commit

Permalink
Merge branch 'master' into 3811-icon-tooltip-sync
Browse files Browse the repository at this point in the history
  • Loading branch information
@asudoh
asudoh authored Aug 27, 2019
2 parents c30b9cc + 6c1221b commit ff643d4
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 32 deletions.
5 changes: 5 additions & 0 deletions packages/components/docs/sass.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10303,6 +10303,11 @@ Data table action styles
.#{$prefix}--toolbar-action:focus:not([disabled]),
.#{$prefix}--toolbar-action:active:not([disabled]) {
@include focus-outline('outline');

&.#{$prefix}--toolbar-search-container-expandable {
// The focus style is handled by search input in it, need to avoid duplicate animation
outline: none;
}
}

.#{$prefix}--toolbar-action ~ .#{$prefix}--btn {
Expand Down
5 changes: 5 additions & 0 deletions packages/components/src/components/data-table/_data-table-action.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@
.#{$prefix}--toolbar-action:focus:not([disabled]),
.#{$prefix}--toolbar-action:active:not([disabled]) {
@include focus-outline('outline');

&.#{$prefix}--toolbar-search-container-expandable {
// The focus style is handled by search input in it, need to avoid duplicate animation
outline: none;
}
}

.#{$prefix}--toolbar-action ~ .#{$prefix}--btn {
Expand Down
13 changes: 7 additions & 6 deletions packages/react/src/components/DataTable/TableToolbarSearch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import cx from 'classnames';
import PropTypes from 'prop-types';
import React, { useRef, useState, useEffect } from 'react';
import React, { useMemo, useRef, useState, useEffect } from 'react';
import { settings } from 'carbon-components';
import Search from '../Search';
import setupGetInstanceId from './tools/instanceId';
Expand All @@ -34,20 +34,21 @@ const TableToolbarSearch = ({
onExpand,
persistent,
persistant,
id = `data-table-search-${getInstanceId()}`,
id,
...rest
}) => {
const { current: controlled } = useRef(expandedProp !== undefined);
const [expandedState, setExpandedState] = useState(defaultExpanded);
const expanded = controlled ? expandedProp : expandedState;
const searchRef = useRef(null);
const [value, setValue] = useState('');
const uniqueId = useMemo(getInstanceId, []);

useEffect(() => {
if (searchRef.current) {
if (!controlled && expandedState && searchRef.current) {
searchRef.current.querySelector('input').focus();
}
});
}, [controlled, expandedState]);

const searchContainerClasses = cx({
[searchContainerClass]: searchContainerClass,
Expand Down Expand Up @@ -77,7 +78,7 @@ const TableToolbarSearch = ({

return (
<div
tabIndex="0"
tabIndex={expandedState ? '-1' : '0'}
role="searchbox"
ref={searchRef}
onClick={event => handleExpand(event, true)}
Expand All @@ -89,7 +90,7 @@ const TableToolbarSearch = ({
small
className={className}
value={value}
id={id}
id={typeof id !== 'undefined' ? id : uniqueId}
aria-hidden={!expanded}
labelText={labelText || t('carbon.table.toolbar.search.label')}
placeHolderText={
Expand Down
34 changes: 29 additions & 5 deletions packages/react/src/components/Tooltip/Tooltip-story.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,12 @@
* LICENSE file in the root directory of this source tree.
*/

import React from 'react';
import React, { useState } from 'react';
import { storiesOf } from '@storybook/react';
import { settings } from 'carbon-components';

import { withKnobs, select, text, number } from '@storybook/addon-knobs';
import Tooltip from '../Tooltip';
import Button from '../Button';

import { OverflowMenuVertical16 } from '@carbon/icons-react';

const { prefix } = settings;
Expand All @@ -22,7 +20,6 @@ const directions = {
'Top (top)': 'top',
'Right (right)': 'right',
};

const props = {
withIcon: () => ({
direction: select('Tooltip direction (direction)', directions, 'bottom'),
Expand Down Expand Up @@ -61,6 +58,32 @@ const props = {
}),
};

function UncontrolledTooltipExample() {
const [value, setValue] = useState(true);
return (
<>
<Button
style={{ padding: '15px 20px', margin: '4px 20px' }}
onClick={() => setValue(false)}>
Hide
</Button>
<Button
style={{ padding: '15px 20px', margin: '4px 20px' }}
onClick={() => setValue(true)}>
Show
</Button>
<div style={{ padding: '15px', margin: '4px 20px' }}>
<Tooltip
triggerText={<div>My text wrapped with tooltip</div>}
open={value}
showIcon={false}>
Some text
</Tooltip>
</div>
</>
);
}

storiesOf('Tooltip', module)
.addDecorator(withKnobs)
.add(
Expand Down Expand Up @@ -178,4 +201,5 @@ storiesOf('Tooltip', module)
`,
},
}
);
)
.add('uncontrolled tooltip', () => <UncontrolledTooltipExample />);
6 changes: 3 additions & 3 deletions packages/react/src/components/Tooltip/Tooltip-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ describe('Tooltip', () => {
const icon = wrapper.find(Information);
icon.simulate('keyDown', { which: 'x' });
// Enzyme doesn't seem to allow state() in a forwardRef-wrapped class component
expect(wrapper.find('Tooltip').instance().state.open).toEqual(false);
expect(wrapper.find('Tooltip').instance().state.open).toBeFalsy();
});

it('A different key press does not change state when custom icon is set', () => {
Expand All @@ -242,13 +242,13 @@ describe('Tooltip', () => {
const icon = wrapper.find('.custom-icon');
icon.simulate('keyDown', { which: 'x' });
// Enzyme doesn't seem to allow state() in a forwardRef-wrapped class component
expect(wrapper.find('Tooltip').instance().state.open).toEqual(false);
expect(wrapper.find('Tooltip').instance().state.open).toBeFalsy();
});

it('should be in a closed state after handleOutsideClick() is invoked', () => {
const rootWrapper = mount(<Tooltip clickToOpen triggerText="Tooltip" />);
// Enzyme doesn't seem to allow state() in a forwardRef-wrapped class component
expect(rootWrapper.find('Tooltip').instance().state.open).toEqual(false);
expect(rootWrapper.find('Tooltip').instance().state.open).toBeFalsy();
// Enzyme doesn't seem to allow setState() in a forwardRef-wrapped class component
rootWrapper
.find('Tooltip')
Expand Down
75 changes: 57 additions & 18 deletions packages/react/src/components/Tooltip/Tooltip.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import ClickListener from '../../internal/ClickListener';
import mergeRefs from '../../tools/mergeRefs';
import { keys, matches as keyDownMatch } from '../../internal/keyboard';
import isRequiredOneOf from '../../prop-types/isRequiredOneOf';
import requiredIfValueExists from '../../prop-types/requiredIfValueExists';
import { useControlledStateWithValue } from '../../internal/FeatureFlags';

const { prefix } = settings;

Expand Down Expand Up @@ -73,7 +75,16 @@ const getMenuOffset = (menuBody, menuDirection) => {
};

class Tooltip extends Component {
state = {};
constructor(props) {
super(props);
this.isControlled = props.open !== undefined;
if (useControlledStateWithValue && this.isControlled) {
// Skips the logic of setting initial state if this component is controlled
return;
}
const open = useControlledStateWithValue ? props.defaultOpen : props.open;
this.state = { open };
}

static propTypes = {
/**
Expand All @@ -86,6 +97,11 @@ class Tooltip extends Component {
*/
tooltipId: PropTypes.string,

/**
* Optional starting value for uncontrolled state
*/
defaultOpen: PropTypes.bool,

/**
* Open/closed state.
*/
Expand Down Expand Up @@ -162,10 +178,19 @@ class Tooltip extends Component {
* Optional prop to specify the tabIndex of the Tooltip
*/
tabIndex: PropTypes.number,

/**
* * the signature of the event handler will be:
* * `onChange(event, { open })` where:
* * `event` is the (React) raw event
* * `open` is the new value
*/
onChange: !useControlledStateWithValue
? PropTypes.func
: requiredIfValueExists(PropTypes.func),
};

static defaultProps = {
open: false,
direction: DIRECTION_BOTTOM,
renderIcon: Information,
showIcon: true,
Expand All @@ -182,7 +207,7 @@ class Tooltip extends Component {

componentDidMount() {
if (!this._debouncedHandleFocus) {
this._debouncedHandleFocus = debounce(this._handleHover, 200);
this._debouncedHandleFocus = debounce(this._handleFocus, 200);
}
requestAnimationFrame(() => {
this.getTriggerPosition();
Expand Down Expand Up @@ -213,6 +238,14 @@ class Tooltip extends Component {
};
}

_handleUserInputOpenClose = (event, { open }) => {
this.setState({ open }, () => {
if (this.props.onChange) {
this.props.onChange(event, { open });
}
});
};

getTriggerPosition = () => {
if (this.triggerEl) {
const triggerPosition = this.triggerEl.getBoundingClientRect();
Expand All @@ -221,14 +254,15 @@ class Tooltip extends Component {
};

/**
* Handles `mouseover`/`mouseout`/`focus`/`blur` event.
* Handles `focus`/`blur` event.
* @param {string} state `over` to show the tooltip, `out` to hide the tooltip.
* @param {Element} [relatedTarget] For handing `mouseout` event, indicates where the mouse pointer is gone.
* @param {Element} [evt] For handing `mouseout` event, indicates where the mouse pointer is gone.
*/
_handleHover = (state, relatedTarget) => {
_handleFocus = (state, evt) => {
const { relatedTarget } = evt;
if (state === 'over') {
this.getTriggerPosition();
this.setState({ open: true });
this._handleUserInputOpenClose(evt, { open: true });
} else {
// Note: SVGElement in IE11 does not have `.contains()`
const shouldPreventClose =
Expand All @@ -238,7 +272,7 @@ class Tooltip extends Component {
this.triggerEl.contains(relatedTarget)) ||
(this._tooltipEl && this._tooltipEl.contains(relatedTarget)));
if (!shouldPreventClose) {
this.setState({ open: false });
this._handleUserInputOpenClose(evt, { open: false });
}
}
};
Expand All @@ -259,6 +293,7 @@ class Tooltip extends Component {
document.body;

handleMouse = evt => {
evt.persist();
const state = {
focus: 'over',
blur: 'out',
Expand All @@ -268,17 +303,19 @@ class Tooltip extends Component {
this._hasContextMenu = evt.type === 'contextmenu';
if (state === 'click') {
evt.stopPropagation();
const shouldOpen = !this.state.open;
const shouldOpen = this.isControlled
? !this.props.open
: !this.state.open;
if (shouldOpen) {
this.getTriggerPosition();
}
this.setState({ open: shouldOpen });
this._handleUserInputOpenClose(evt, { open: shouldOpen });
} else if (
state &&
(state !== 'out' || !hadContextMenu) &&
this._debouncedHandleFocus
) {
this._debouncedHandleFocus(state, evt.relatedTarget);
this._debouncedHandleFocus(state, evt);
}
};

Expand All @@ -289,30 +326,32 @@ class Tooltip extends Component {
this._tooltipEl &&
this._tooltipEl.contains(evt.target);
if (!shouldPreventClose) {
this.setState({ open: false });
this._handleUserInputOpenClose(evt, { open: false });
}
};

handleKeyPress = event => {
if (keyDownMatch(event, [keys.Escape])) {
event.stopPropagation();
this.setState({ open: false });
this._handleUserInputOpenClose(event, { open: false });
}

if (keyDownMatch(event, [keys.Enter, keys.Space])) {
event.stopPropagation();
const shouldOpen = !this.state.open;
const shouldOpen = this.isControlled
? !this.props.open
: !this.state.open;
if (shouldOpen) {
this.getTriggerPosition();
}
this.setState({ open: shouldOpen });
this._handleUserInputOpenClose(event, { open: shouldOpen });
}
};

handleEscKeyPress = event => {
const { open } = this.state;
const { open } = this.isControlled ? this.props : this.state;
if (open && keyDownMatch(event, [keys.Escape])) {
return this.setState({ open: false });
return this._handleUserInputOpenClose(event, { open: false });
}
};

Expand Down Expand Up @@ -343,7 +382,7 @@ class Tooltip extends Component {
...other
} = this.props;

const { open } = this.state;
const { open } = this.isControlled ? this.props : this.state;

const tooltipClasses = classNames(
`${prefix}--tooltip`,
Expand Down

0 comments on commit ff643d4

Please sign in to comment.