Skip to content

Commit

Permalink
fix(header): use valid li > a html nesting + a11y improvements (#1015)
Browse files Browse the repository at this point in the history 
* fix: invalid html

* fix: class naming through html reorganization

* refactor: updating unit testing snapshots
  • Loading branch information
@marvinLaubenstein
marvinLaubenstein authored May 2, 2022
1 parent ddbd0d1 commit 614f83d
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 87 deletions.
24 changes: 12 additions & 12 deletions .../telekom/app-navigation-main-mobile/__snapshots__/app-navigation-main-mobile.spec.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ exports[`AppNavigationMainMobile should handle menuitem 1`] = `
</div>
</a>
<ul class="main-navigation-mobile__child-menu-items">
<a aria-current="true" aria-haspopup="false" class="main-navigation-mobile__child-menu-item-link selected" href="#" tabindex="0">
<li class="main-navigation-mobile__child-menu-item">
<li class="main-navigation-mobile__child-menu-item">
<a aria-current="true" aria-haspopup="false" class="main-navigation-mobile__child-menu-item-link selected" href="#" tabindex="0">
<div class="main-navigation-mobile__child-menu-item-wrapper">
<span>
menuItem1-1
Expand All @@ -25,13 +25,13 @@ exports[`AppNavigationMainMobile should handle menuitem 1`] = `
active
</span>
</div>
</li>
</a>
</a>
</li>
</ul>
</div>
<ul class="main-navigation-mobile__main-menu">
<a aria-current="true" aria-haspopup="true" class="main-navigation-mobile__item-link main-navigation-mobile__item-link--selected" href="#" tabindex="-1">
<li class="main-navigation-mobile__item">
<li class="main-navigation-mobile__item main-navigation-mobile__item--selected">
<a aria-current="true" aria-haspopup="true" class="main-navigation-mobile__item-link main-navigation-mobile__item-link--selected" href="#" tabindex="-1">
<div class="main-navigation-mobile__item-wrapper">
<span>
nav1
Expand All @@ -41,8 +41,8 @@ exports[`AppNavigationMainMobile should handle menuitem 1`] = `
</span>
<scale-icon-navigation-right></scale-icon-navigation-right>
</div>
</li>
</a>
</a>
</li>
</ul>
</div>
</app-navigation-main-mobile>
Expand All @@ -53,15 +53,15 @@ exports[`AppNavigationMainMobile should handle without childs 1`] = `
<div class="main-navigation-mobile">
<div></div>
<ul class="main-navigation-mobile__main-menu">
<a aria-current="false" aria-haspopup="false" class="main-navigation-mobile__item-link" href="#" tabindex="0">
<li class="main-navigation-mobile__item">
<li class="main-navigation-mobile__item">
<a aria-current="false" aria-haspopup="false" class="main-navigation-mobile__item-link" href="#" tabindex="0">
<div class="main-navigation-mobile__item-wrapper">
<span>
nav1
</span>
</div>
</li>
</a>
</a>
</li>
</ul>
</div>
</app-navigation-main-mobile>
Expand Down
26 changes: 13 additions & 13 deletions ...mponents/src/components/telekom/app-navigation-main-mobile/app-navigation-main-mobile.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,8 @@ app-navigation-main-mobile {
line-height: var(--line-height);
}

.main-navigation-mobile__item-wrapper {
box-sizing: border-box;
width: calc(100% - 34px);
display: flex;
justify-content: space-between;
margin-left: 34px;
border-bottom: var(--border-bottom);
padding-right: 34px;
align-items: center;
.main-navigation-mobile__item--selected {
border-right: 2px solid var(--color-selected);
}

.main-navigation-mobile__item-link {
Expand All @@ -68,14 +61,21 @@ app-navigation-main-mobile {
transition: var(--transition);
}

.main-navigation-mobile__item-link--selected .main-navigation-mobile__item {
border-right: 2px solid var(--color-selected);
}

.main-navigation-mobile__item-link--selected svg {
margin-right: -2px;
}

.main-navigation-mobile__item-wrapper {
box-sizing: border-box;
width: calc(100% - 34px);
display: flex;
justify-content: space-between;
margin-left: 34px;
border-bottom: var(--border-bottom);
padding-right: 34px;
align-items: center;
}

.main-navigation-mobile__child-menu {
position: absolute;
top: 0;
Expand Down
130 changes: 68 additions & 62 deletions ...mponents/src/components/telekom/app-navigation-main-mobile/app-navigation-main-mobile.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,45 +135,45 @@ export class MainNavigationMobile {
}}
>
{section.children.map((child) => (
<a
aria-current={isActive(child) ? 'true' : 'false'}
aria-haspopup={child.children ? 'true' : 'false'}
class={`main-navigation-mobile__child-menu-item-link ${
isActive(child) ? 'selected' : ''
}`}
href={child.href || 'javascript:void(0);'}
tabIndex={0}
onClick={(event) => {
this.handleSelect(event, child);
}}
onKeyDown={(event) => {
if (['Enter', ' '].includes(event.key)) {
<li class="main-navigation-mobile__child-menu-item">
<a
aria-current={isActive(child) ? 'true' : 'false'}
aria-haspopup={child.children ? 'true' : 'false'}
class={`main-navigation-mobile__child-menu-item-link ${
isActive(child) ? 'selected' : ''
}`}
href={child.href || 'javascript:void(0);'}
tabIndex={0}
onClick={(event) => {
this.handleSelect(event, child);
setTimeout(() => {
// focus first child menu item link to ease tab navigation
const firstChildren = this.childrenWrapper.querySelector(
'a'
);
if (firstChildren) {
this.childrenWrapper.querySelector('a').focus();
}
});
}
if (['Escape', 'Esc'].includes(event.key)) {
this.hide();
}
}}
>
<li class="main-navigation-mobile__child-menu-item">
}}
onKeyDown={(event) => {
if (['Enter', ' '].includes(event.key)) {
this.handleSelect(event, child);
setTimeout(() => {
// focus first child menu item link to ease tab navigation
const firstChildren = this.childrenWrapper.querySelector(
'a'
);
if (firstChildren) {
this.childrenWrapper.querySelector('a').focus();
}
});
}
if (['Escape', 'Esc'].includes(event.key)) {
this.hide();
}
}}
>
<div class="main-navigation-mobile__child-menu-item-wrapper">
<span>{child.name}</span>
{isActive(child) && <span class="sr-only">active</span>}
{child.children && (
<scale-icon-navigation-right></scale-icon-navigation-right>
)}
</div>
</li>
</a>
</a>
</li>
))}
</ul>
</div>
Expand All @@ -194,48 +194,54 @@ export class MainNavigationMobile {
}}
>
{(this.navigation || []).map((item) => (
<a
aria-current={isActive(item.id) ? 'true' : 'false'}
aria-haspopup={item.children ? 'true' : 'false'}
class={`main-navigation-mobile__item-link${
<li
class={`main-navigation-mobile__item${
isActive(item.id)
? ' main-navigation-mobile__item-link--selected'
? ' main-navigation-mobile__item--selected'
: ''
}`}
href={item.href || 'javascript:void(0);'}
onClick={(event) => {
this.handleSelect(event, item);
}}
onKeyDown={(event) => {
if (['Enter', ' '].includes(event.key)) {
this.handleSelect(event, item);
setTimeout(() => {
// focus first child menu item link to ease tab navigation
const firstChildren = this.childrenWrapper.querySelector(
'a'
);
if (firstChildren) {
this.childrenWrapper.querySelector('a').focus();
}
});
}
if (['Escape', 'Esc'].includes(event.key)) {
this.hide();
}
}}
// hide from tab navigation when on childMenuPage
tabIndex={this.selected ? -1 : 0}
>
<li class="main-navigation-mobile__item">
<a
aria-current={isActive(item.id) ? 'true' : 'false'}
aria-haspopup={item.children ? 'true' : 'false'}
class={`main-navigation-mobile__item-link${
isActive(item.id)
? ' main-navigation-mobile__item-link--selected'
: ''
}`}
href={item.href || 'javascript:void(0);'}
onClick={(event) => {
this.handleSelect(event, item);
}}
onKeyDown={(event) => {
if (['Enter', ' '].includes(event.key)) {
this.handleSelect(event, item);
setTimeout(() => {
// focus first child menu item link to ease tab navigation
const firstChildren = this.childrenWrapper.querySelector(
'a'
);
if (firstChildren) {
this.childrenWrapper.querySelector('a').focus();
}
});
}
if (['Escape', 'Esc'].includes(event.key)) {
this.hide();
}
}}
// hide from tab navigation when on childMenuPage
tabIndex={this.selected ? -1 : 0}
>
<div class="main-navigation-mobile__item-wrapper">
<span>{item.name}</span>
{isActive(item.id) && <span class="sr-only">active</span>}
{item.children && (
<scale-icon-navigation-right></scale-icon-navigation-right>
)}
</div>
</li>
</a>
</a>
</li>
))}
</ul>
</div>
Expand Down

0 comments on commit 614f83d

Please sign in to comment.