Skip to content

Commit

Permalink
fix: clean up SelectMenu behaviors
Browse files Browse the repository at this point in the history
The SelectMenu had several odd behaviors:

The trigger button had no max size and would wrap text, causing inconvenience.
Giving each menuItem a unique random uuid created difficulty because of the birthday paradox. Changed to use refs instead in much cleaner form.
The popper could shift left/right if the size of the trigger button changed.
offset sometimes wasn't perfect.
  • Loading branch information
connorhaugh committed Sep 10, 2021
1 parent f9996ec commit 91c7a50
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 62 deletions.
12 changes: 6 additions & 6 deletions src/Menu/Menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,19 @@
border-radius: 0.25em;
box-shadow: $box-shadow;
background-color: $white;
border-color: transparent;
overflow: scroll;
min-width: 288px;
max-width: 450px;
overflow: auto;
max-height: 264px;
}
.pgn__menu-select-popup{
max-height: 267px;

color: $btn-tertiary-bg;
}
.pgn__menu-select-trigger-btn{
max-width: 450px;
white-space: nowrap;
overflow: hidden;
text-overflow:ellipsis;
background: $white;
border: 1px solid #F2F0EF;
box-sizing: border-box;
color: $dark;

Expand Down
115 changes: 60 additions & 55 deletions src/Menu/SelectMenu.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,72 +14,93 @@ const SelectMenu = ({
...props
}) => {
const triggerTarget = React.useRef(null);
const triggerRef = React.useRef(null);
const itemsCollection = React.useMemo(
() => Array.from({ length: children.length }).map(() => React.createRef()),
[],
);

const className = classNames(props.className, 'pgn__menu-select');
const [selected, setSelected] = useState();
function defaultIndex() {
for (let i = 0; i < children.length; i++) {
if (children[i].props && children[i].props.defaultSelected) {
return i;
}
}
return undefined;
}
const [selected, setSelected] = useState(defaultIndex());

const [isOpen, open, close] = useToggle(false);
const [vertOffset, setOffset] = useState(0);
const link = isLink; // allow inline link styling

const createMenuItems = () => {
const elements = [];
React.Children.map(children, (child) => {
const newProps = {
onClick(e) {
if (child.props.onClick) {
child.props.onClick(e);
}
setSelected(children.indexOf(child));
close();
triggerTarget.current.focus();
},
id: `${children.indexOf(child).toString()}_pgn__menu-item`,
role: 'link',
};
if (selected === children.indexOf(child)) {
newProps['aria-current'] = 'page';
}
elements.push(
React.cloneElement(child, newProps),
);
});
return elements;
};

const link = isLink; // allow inline link styling
const prevOpenRef = React.useRef();
useEffect(() => {
// logic to always center the selected item.
if (isOpen && selected) {
const index = parseInt(selected.id.slice(0, selected.id.indexOf('_')), 10);
const numItems = children.length;

const boundingRect = document.getElementById(selected.id).parentElement.getBoundingClientRect();
const boundingRect = itemsCollection[selected].current.parentElement.getBoundingClientRect();
if (boundingRect.bottom >= window.innerHeight - 150 || boundingRect.top <= 150) {
setOffset(0); // if too close to the edge, don't do centering fancyness
} else {
switch (true) {
case numItems < 6: {
// on small lists, center each element
setOffset(
parseInt(selected.id.slice(0, selected.id.indexOf('_') + 1), 10) * -48 + 20,
(selected) * -48,
);
break;
}
case index < 2: {
case selected < 2: {
// On first two elements, set offset based on position
setOffset((index) * -48);
setOffset((selected) * -48);
break;
}
case numItems - index < 3: {
case numItems - selected < 3: {
// on n-1 and n-2 elelements, set offset to put most modal elements on top.
setOffset((6 - (numItems - index)) * -48);
setOffset((6 - (numItems - selected)) * -48);
break;
}
case index > 1 && numItems - index > 2: {
case selected > 1 && numItems - selected > 2: {
// on "middle elements", set offset to center of block and scroll to center
document.getElementById(selected.id).scrollIntoView({
itemsCollection[selected].current.children[0].scrollIntoView({
block: 'center',
});
setOffset(-125);
setOffset(2 * -48);
break;
}
default: break;
}
}
}
// set focus on open
if (isOpen && !prevOpenRef.current) {
if (selected) { document.getElementById(selected.id).focus(); } else {
React.Children.forEach(children, (child) => {
if (child.props.defaultSelected) {
const buttonTags = document.getElementsByTagName('button');
for (let i = 0; i < buttonTags.length; i++) {
if (buttonTags[i].textContent === child.props.children) {
setSelected(buttonTags[i]);
buttonTags[i].focus({
preventScroll: true,
});
break;
}
}
}
});
}
if (isOpen && !prevOpenRef.current && selected) {
itemsCollection[selected].current.children[0].focus({ preventScroll: (defaultIndex() === selected) });
}
prevOpenRef.current = isOpen;
});
Expand All @@ -91,16 +112,15 @@ const SelectMenu = ({
className,
},
<>
<span ref={triggerTarget} />
<Button
aria-haspopup="true"
aria-expanded={isOpen}
ref={triggerRef}
ref={triggerTarget}
className="pgn__menu-select-trigger-btn"
variant={link ? 'link' : 'tertiary'}
iconAfter={link ? undefined : ExpandMore}
onClick={open}
>{ selected ? selected.innerText : defaultMessage}
>{ selected !== defaultIndex() ? children[selected].props.children : defaultMessage}
</Button>
<div className="pgn__menu-select-popup">
<ModalPopup
Expand All @@ -118,33 +138,18 @@ const SelectMenu = ({
name: 'offset',
options: {
enabled: true,
offset: [vertOffset, 0],
offset: [vertOffset, triggerTarget.current ? -1 * triggerTarget.current.offsetWidth : 0],
},
},
]
}
>
<Menu aria-label="Select Menu">
{
React.Children.map(children, (child) => {
const newProps = {
onClick(e) {
if (child.props.onClick) {
child.props.onClick(e);
}
setSelected(e.target);
close();
triggerRef.current.focus();
},
id: `${children.indexOf(child).toString()}_pgn__menu-item`,
role: 'link',
};
if (selected && selected.id === `${children.indexOf(child).toString()}_pgn__menu-item`) {
newProps['aria-current'] = 'page';
}
return React.cloneElement(child, newProps);
})
}
{createMenuItems().map((child, index) => (
<div ref={itemsCollection[index]}>
{child}
</div>
))}
</Menu>
</ModalPopup>
</div>
Expand Down
1 change: 0 additions & 1 deletion src/Menu/__snapshots__/SelectMenu.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ exports[`Rendering Beahvior Renders as expected 1`] = `
<pgn__menu-select
className="pgn__menu-select"
>
<span />
<button
aria-expanded={false}
aria-haspopup="true"
Expand Down

0 comments on commit 91c7a50

Please sign in to comment.