Commit b2887be5 authored by Robert Knight's avatar Robert Knight

Fix visibility of submenu item focus rings

Item submenus inside the `Menu` component are wrappd in a `Slider` to
enable an animated expand/collapse of the submenu. The `Slider` used
`overflow: hidden` to hide any content while collapsed or transitioning.
However this had the unintended effect of clipping focus rings which
extended beyond the bounds of the content.

Fix the issue by changing the `overflow` property to `visible` once the
slider's content is fully visible.
parent 0eda1d11
......@@ -69,6 +69,8 @@ export default function Slider({ children, visible }) {
}
}, [setContainerHeight, visible]);
const isFullyVisible = containerHeight === 'auto';
return (
<div
// nb. Preact uses "ontransitionend" rather than "onTransitionEnd".
......@@ -80,7 +82,11 @@ export default function Slider({ children, visible }) {
style={{
display: contentVisible ? '' : 'none',
height: containerHeight,
overflow: 'hidden',
// When the slider is fully open, overflow is made visible so that
// focus rings, which may extend outside the bounds of the slider content,
// are visible.
overflow: isFullyVisible ? 'visible' : 'hidden',
transition: `height 0.15s ease-in`,
}}
>
......
......@@ -77,6 +77,23 @@ describe('Slider', () => {
assert.equal(containerStyle.height, 'auto');
});
it('should hide overflowing content when not fully visible', () => {
// When fully collapsed, overflow should be hidden.
const wrapper = createSlider({ visible: false });
let containerStyle = wrapper.getDOMNode().style;
assert.equal(containerStyle.overflow, 'hidden');
// When starting to expand, or when collapsing, overflow should also be hidden.
wrapper.setProps({ visible: true });
assert.equal(containerStyle.overflow, 'hidden');
// When fully visible, we make overflow visible to make focus rings or
// other content which extends beyond the bounds of the slider visible.
wrapper.find('div').first().simulate('transitionend');
containerStyle = wrapper.getDOMNode().style;
assert.equal(containerStyle.overflow, 'visible');
});
it('should stop rendering content when a collapse transition finishes', () => {
const wrapper = createSlider({ visible: true });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment