fix(overlays): Allow elements with display:contents to be focusable in keyboard event trapping (#1709)
* fix: isVisible check on elements with display:contents * Create eight-years-pump.md * chore: adds additional test
This commit is contained in:
parent
ae53377ce4
commit
915de370d7
4 changed files with 37 additions and 3 deletions
5
.changeset/eight-years-pump.md
Normal file
5
.changeset/eight-years-pump.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@lion/overlays": patch
|
||||
---
|
||||
|
||||
fix: isVisible check on elements with display:contents
|
||||
|
|
@ -4,6 +4,11 @@
|
|||
const hasStyleVisibility = ({ visibility, display }) =>
|
||||
visibility !== 'hidden' && display !== 'none';
|
||||
|
||||
/**
|
||||
* @param {CSSStyleDeclaration} styles
|
||||
*/
|
||||
const isDisplayContents = ({ display }) => display === 'contents';
|
||||
|
||||
/**
|
||||
* @param {HTMLElement} element
|
||||
* @returns {boolean} Whether the element is visible
|
||||
|
|
@ -24,12 +29,20 @@ export function isVisible(element) {
|
|||
return false;
|
||||
}
|
||||
|
||||
const computedStyle = window.getComputedStyle(element);
|
||||
|
||||
// Check computed styles
|
||||
// matches display: none, visbility: hidden on element and visibility: hidden from parent
|
||||
if (!hasStyleVisibility(window.getComputedStyle(element))) {
|
||||
// matches display: none, visibility: hidden on element and visibility: hidden from parent
|
||||
if (!hasStyleVisibility(computedStyle)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Allow element that delegates layout (i.e. display: contents)
|
||||
// matches display: contents
|
||||
if (isDisplayContents(computedStyle)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// display: none is not inherited, so finally check if element has calculated width or height
|
||||
// matches display: none from parent
|
||||
return !!(element.offsetWidth || element.offsetHeight || element.getClientRects().length);
|
||||
|
|
|
|||
|
|
@ -53,12 +53,16 @@ describe('getFocusableElements()', () => {
|
|||
<input id='el4'>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div style='display: contents;'>
|
||||
<button id='el5'></button>
|
||||
</div>
|
||||
</div>
|
||||
`);
|
||||
const nodes = getFocusableElements(element);
|
||||
const ids = nodes.map(n => n.id);
|
||||
|
||||
expect(ids).eql(['el1', 'el2', 'el3', 'el4']);
|
||||
expect(ids).eql(['el1', 'el2', 'el3', 'el4', 'el5']);
|
||||
});
|
||||
|
||||
it('skips elements that should not receive focus', async () => {
|
||||
|
|
|
|||
|
|
@ -142,4 +142,16 @@ describe('isVisible()', () => {
|
|||
const target = /** @type {HTMLElement} */ (element.querySelector('#target'));
|
||||
expect(isVisible(target)).to.equal(false);
|
||||
});
|
||||
|
||||
it('returns true when display contents', async () => {
|
||||
const element = /** @type {HTMLElement} */ (
|
||||
await fixture(`
|
||||
<div style="display: contents;">
|
||||
<div style="width:10px; height:10px;"></div>
|
||||
</div>
|
||||
`)
|
||||
);
|
||||
|
||||
expect(isVisible(element)).to.equal(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue