Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@
"react-dom": "^16.6.3"
},
"dependencies": {
"classnames": "^2.2.6"
"classnames": "^2.2.6",
"use-onclickoutside": "^0.3.1"
},
"husky": {
"hooks": {
Expand Down
2 changes: 1 addition & 1 deletion src/Choice Tile/index.js → src/ChoiceTile/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ ChoiceTile.propTypes = {
invalid: PropTypes.bool,
name: PropTypes.string.isRequired,
onChange: PropTypes.func,
size: PropTypes.oneOf(SIZES),
size: PropTypes.oneOf(Object.keys(SIZES)),
subdued: PropTypes.bool,
title: PropTypes.string.isRequired,
};
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion src/Drawer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Drawer.propTypes = {
onClose: PropTypes.func,
onOpen: PropTypes.func,
open: PropTypes.bool,
size: PropTypes.oneOf(SIZES),
size: PropTypes.oneOf(Object.keys(SIZES)),
};

Drawer.defaultProps = {
Expand Down
2 changes: 1 addition & 1 deletion src/Modal/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const Modal = ({ onOpen, onClose, className, open, size, children, ...rest }) =>

Modal.propTypes = {
id: PropTypes.string,
size: PropTypes.oneOf(SIZES),
size: PropTypes.oneOf(Object.keys(SIZES)),
children: PropTypes.node.isRequired,
className: PropTypes.string,
open: PropTypes.bool,
Expand Down
61 changes: 61 additions & 0 deletions src/Search/SearchAssist.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import PropTypes from 'prop-types';
import React, { useState, useRef } from 'react';
import Search from './index';
import SearchAssistance from './SearchAssistance';
import { wcBool } from '../utils';
import { POSITIONS } from '../constants';
import useClickOutside from 'use-onclickoutside';

/**
* @see https://helixdesignsystem.github.io/helix-ui/components/search/
*/
const SearchAssist = ({ children, onFocus, onBlur, position, ...rest }) => {
const [open, setOpen] = useState(false);

const searchRef = useRef();
useClickOutside(searchRef, () => setOpen(false));

const hasChildren = React.Children.toArray(children).filter((c) => c).length > 0;

return (
<div ref={searchRef}>
<Search
{...rest}
onFocus={(e) => {
setOpen(true);
onFocus && onFocus(e);
}}
wrapperId={`${rest.id}-hx-search-control`}
/>
{hasChildren && (
<SearchAssistance
relativeTo={`${rest.id}-hx-search-control`}
open={wcBool(open)}
position={position}
onClick={() => setOpen(false)}
>
{children}
</SearchAssistance>
)}
</div>
);
};

SearchAssist.propTypes = {
children: PropTypes.node.isRequired,
position: PropTypes.oneOf(POSITIONS),
className: PropTypes.string,
clearLabel: PropTypes.string,
label: PropTypes.string,
id: PropTypes.string.isRequired,
wrapperId: PropTypes.string,
disabled: PropTypes.bool,
onChange: PropTypes.func,
onFocus: PropTypes.func,
onBlur: PropTypes.func,
onClear: PropTypes.func,
optional: PropTypes.bool,
required: PropTypes.bool,
};

export default SearchAssist;
23 changes: 23 additions & 0 deletions src/Search/SearchAssistance.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import PropTypes from 'prop-types';
import React from 'react';
import { POSITIONS } from '../constants';

/**
* @see https://helixdesignsystem.github.io/helix-ui/elements/hx-search-assistance/
*/
const SearchAssistance = ({ children, className, relativeTo, ...rest }) => {
return (
<hx-search-assistance class={className} relative-to={relativeTo} {...rest}>
{children}
</hx-search-assistance>
);
};

SearchAssistance.propTypes = {
children: PropTypes.node.isRequired,
className: PropTypes.string,
relativeTo: PropTypes.string.isRequired,
position: PropTypes.oneOf(POSITIONS),
};

export default SearchAssistance;
64 changes: 64 additions & 0 deletions src/Search/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import classnames from 'classnames';
import PropTypes from 'prop-types';
import React from 'react';
import Icon from '../Icon';
import { wcBool } from '../utils';

/**
* @see https://helixdesignsystem.github.io/helix-ui/components/search/
*/
const Search = ({
children,
disabled,
id,
label,
className,
clearLabel,
onClear,
optional,
required,
wrapperId,
...rest
}) => {
return (
<hx-search-control class={className} id={wrapperId}>
Copy link
Contributor Author

@nicko-winner nicko-winner Jun 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have prop called wrapperId because id is being passed to the input. This seems to be the convention for input that @michaelmang started. Note: <.*-control /> elements don't seem to need id's as frequently as the inputs, search may be the only one control wrapper that needs and ID?

<input id={id} {...rest} disabled={wcBool(disabled)} type="search" />
<button type="button" className="hxClear" aria-label={clearLabel} hidden onClick={onClear}>
<Icon type="times" />
</button>
<hx-search></hx-search>
{label && (
<label
className={classnames({
hxOptional: optional,
hxRequired: required,
})}
htmlFor={id}
>
{label}
</label>
)}
</hx-search-control>
);
};

Search.propTypes = {
className: PropTypes.string,
clearLabel: PropTypes.string,
label: PropTypes.string,
id: PropTypes.string.isRequired,
wrapperId: PropTypes.string,
disabled: PropTypes.bool,
onChange: PropTypes.func,
onFocus: PropTypes.func,
onBlur: PropTypes.func,
onClear: PropTypes.func,
optional: PropTypes.bool,
required: PropTypes.bool,
};

Search.defaultProps = {
clearLabel: 'Clear search',
};

export default Search;
72 changes: 72 additions & 0 deletions src/Search/stories.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { action } from '@storybook/addon-actions';
import { boolean, text, select } from '@storybook/addon-knobs';
import { storiesOf } from '@storybook/react';
import React, { useState } from 'react';
import Search from '../Search';
import SearchAssist from './SearchAssist';
import { InputContainer } from '../storyUtils';
import { POSITIONS } from '../constants';

storiesOf('Search', module)
.add('All Knobs', () => {
let disabled = boolean('disabled', false);
let label = text('label', '');
let optional = boolean('optional', false);
let required = boolean('required', false);
let position = select('positions', POSITIONS, 'bottom-center');

return (
<InputContainer>
<Search
id="my-search"
{...(disabled && { disabled })}
{...(label && { label })}
{...(optional && { optional })}
{...(required && { required })}
{...(position && { position })}
onChange={action('change')}
autocomplete="off"
/>
</InputContainer>
);
})
.add('SearchAssist', () => {
let disabled = boolean('disabled', false);
let label = text('label', 'Search');
let optional = boolean('optional', false);
let required = boolean('required', false);
let position = select('positions', POSITIONS, 'bottom-center');
const [value, setValue] = useState('');

return (
<InputContainer>
<SearchAssist
id="my-assisted-search"
onChange={(e) => setValue(e.target.value)}
value={value}
onClear={(e) => {
action('onClear');
setValue('');
}}
onFocus={(e) => action('onFocus')}
onBlur={(e) => action('onBlur')}
autocomplete="off"
{...(disabled && { disabled })}
{...(label && { label })}
{...(optional && { optional })}
{...(required && { required })}
{...(position && { position })}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears the position is getting transposed for bottom-center and top-center...I wonder if this is something in HelixUI logic?

screenshot bottom-center:

Screen Shot 2020-06-16 at 11 27 11 AM

screenshot top-center:

Screen Shot 2020-06-16 at 11 26 30 AM

Copy link
Contributor Author

@nicko-winner nicko-winner Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an issue in the helix positioning logic. We are sending the correct props. The storybook controls on the bottom for knobs are pushing up against the bottom of the search suggestion box. I think this issue exists for all helix positionable elements.

Examples from official helix docs:
2020-06-16_17-16-34

Popover is a good place to reproduce this issue inside helix:
https://helixdesignsystem.github.io/helix-ui/components/popover/

make your window smaller so helix places the popover, on top, then choose the top-right option on popover, and it will reverse the logic.
2020-06-16_17-22-05

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok makes sense, I see it's the size of the Storybook preview pane for me. Yes, that's the default behavior for HelixUI components with the positioning logic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw what are your thoughts the positioning behavior? Any tweaks needed?

Copy link
Contributor Author

@nicko-winner nicko-winner Jun 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the fix in helix might look like this: Before trying to reposition an item make sure there is more space in the direction the menu wants to reposition in. Maybe only trigger the reposition if there is at least 50-100px more space in the direction it wants to reposition into. If there is not, then keep its existing position. That may help menus from prepositioning from a bad fit to an unintended worse fit. Or maybe there is an option to disable position changing for use cases that do not need that behavior?

Copy link
Member

@100stacks 100stacks Jun 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's really great feedback. Most of our use cases were based on users scrolling, and the direction they are scrolling.

Here's the collision detection logic:

https://github.com/HelixDesignSystem/helix-ui/blob/master/src/utils/alignment/index.js#L229-L300

Good suggestion for allowing implementors an option to disable the collision detection.

>
<header>Search for "{value}"</header>
<section>
<header>Category Header</header>
{POSITIONS.filter((p) => p.search(value) !== -1).map((item) => (
<button className="hxSearchSuggestion" key={item} onClick={() => setValue(item)}>
Here is a possible match: {item}
</button>
))}
</section>
</SearchAssist>
</InputContainer>
);
});
10 changes: 8 additions & 2 deletions src/index.mjs
Original file line number Diff line number Diff line change
@@ -1,20 +1,26 @@
/* Export helix-react definition */
import Button from './Button';
import Alert from './Alert';
import Button from './Button';
import ChoiceTile from './ChoiceTile';
import Drawer from './Drawer';
import Icon from './Icon';
import Modal from './Modal';
import Popover from './Popover';
import Tooltip from './Tooltip';
import Select from './Select';
import Search from './Search';
import SearchAssist from './Search/SearchAssist';

export default {
Button,
Alert,
Button,
ChoiceTile,
Drawer,
Icon,
Modal,
Popover,
Select,
Tooltip,
Search,
SearchAssist
};
4 changes: 4 additions & 0 deletions src/storyUtils.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import React from 'react';

export const getShortText = () => `lorem ipsum dolor sir amet`;

export const getLongText = () => `
Expand All @@ -13,3 +15,5 @@ export const getLongText = () => `
elementum integer enim neque volutpat. Etiam sit amet nisl purus in mollis nunc. Diam sit amet
nisl suscipit. Nulla pharetra diam sit amet nisl. Arcu odio ut sem nulla.
`;

export const InputContainer = ({ children }) => <div style={{ width: 500 }}>{children}</div>;
18 changes: 18 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1896,6 +1896,11 @@ aproba@^1.0.3, aproba@^1.1.1:
version "1.2.0"
resolved "https://registry.yarnpkg.com/aproba/-/aproba-1.2.0.tgz#6802e6264efd18c790a1b0d517f0f2627bf2c94a"

are-passive-events-supported@^1.1.0:
version "1.1.1"
resolved "https://registry.yarnpkg.com/are-passive-events-supported/-/are-passive-events-supported-1.1.1.tgz#3db180a1753a2186a2de50a32cded3ac0979f5dc"
integrity sha512-5wnvlvB/dTbfrCvJ027Y4L4gW/6Mwoy1uFSavney0YO++GU+0e/flnjiBBwH+1kh7xNCgCOGvmJC3s32joYbww==

are-we-there-yet@~1.1.2:
version "1.1.5"
resolved "https://registry.yarnpkg.com/are-we-there-yet/-/are-we-there-yet-1.1.5.tgz#4b35c2944f062a8bfcda66410760350fe9ddfc21"
Expand Down Expand Up @@ -7461,6 +7466,19 @@ use-callback-ref@^1.2.1:
version "1.2.1"
resolved "https://registry.yarnpkg.com/use-callback-ref/-/use-callback-ref-1.2.1.tgz#898759ccb9e14be6c7a860abafa3ffbd826c89bb"

use-latest@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/use-latest/-/use-latest-1.0.0.tgz#c86d2e4893b15f27def69da574a47136d107facb"
integrity sha512-CxmFi75KTXeTIBlZq3LhJ4Hz98pCaRKZHCpnbiaEHIr5QnuHvH8lKYoluPBt/ik7j/hFVPB8K3WqF6mQvLyQTg==

use-onclickoutside@^0.3.1:
version "0.3.1"
resolved "https://registry.yarnpkg.com/use-onclickoutside/-/use-onclickoutside-0.3.1.tgz#fdd723a6a499046b6bc761e4a03af432eee5917b"
integrity sha512-aahvbW5+G0XJfzj31FJeLsvc6qdKbzeTsQ8EtkHHq5qTg6bm/qkJeKLcgrpnYeHDDbd7uyhImLGdkbM9BRzOHQ==
dependencies:
are-passive-events-supported "^1.1.0"
use-latest "^1.0.0"

use-sidecar@^1.0.1:
version "1.0.2"
resolved "https://registry.yarnpkg.com/use-sidecar/-/use-sidecar-1.0.2.tgz#e72f582a75842f7de4ef8becd6235a4720ad8af6"
Expand Down