Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: keyboard navigation when using position fixed #5792

Merged
merged 1 commit into from
Apr 27, 2023

Conversation

sissbruecker
Copy link
Contributor

Description

Keyboard navigation for tab bar is currently broken when the element is within the app layout's drawer. The cause is the following check which incorrectly identifies all tabs as hidden:

// `offsetParent` is `null` when the element itself
// or one of its ancestors is hidden with `display: none`.
// https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/offsetParent
if (element.offsetParent === null) {
return true;
}

The problem is that offsetParent is also null if the element is using fixed positioning. It can also be null for nested elements in a container with fixed positioning, such as for the tabs being in the app layouts drawer.

This change adds an additional check to verify that items have no client size which would be another indicator that display: none is being used in the hierarchy.

Fixes #5706

Type of change

  • Bugfix

@sonarcloud
Copy link

sonarcloud bot commented Apr 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication


it('should return false when using fixed positioning', () => {
element.style.position = 'fixed';
expect(isElementHidden(element)).to.be.false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also wanted to check the child's visibility here, but in case of these nested divs, the child's offset parent is actually the parent div. In case of tabs being used within the app layouts drawer there must be some additional detail that causes the offset parent to become null.

@sissbruecker sissbruecker merged commit 2aecbfb into main Apr 27, 2023
9 checks passed
@sissbruecker sissbruecker deleted the fix/keyboard_navigation_when_position_fixed branch April 27, 2023 07:58
sissbruecker added a commit that referenced this pull request Apr 27, 2023
Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
sissbruecker added a commit that referenced this pull request Apr 27, 2023
Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keyboard navigation broken in vertical Tabs
3 participants