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(internet-header): Trap focus is not working on overlays and missing in search overlay #2922

Merged
merged 13 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/spicy-nails-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@swisspost/internet-header': patch
---

Fixed focus trap on overlay of the breadcrumb.
Added a focus trap on search overlay
4 changes: 2 additions & 2 deletions packages/internet-header/cypress/e2e/breadcrumbs.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ describe('breadcrumb', () => {
cy.get('@breadcrumbs')
.find('div.overlay')
.should('exist')
.find('div.overlay-container')
.should('have.class', 'loaded');
.find('[role=dialog].loaded', { timeout: 30000 })
.should('exist');

Comment on lines -17 to 19
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A better test would be to check if the element is visible or not

cy.get('@breadcrumbs')
.find('div.overlay')
Expand Down
2 changes: 1 addition & 1 deletion packages/internet-header/cypress/e2e/header.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('header', () => {

context('initial state', () => {
it('renders', () => {
cy.get('swisspost-internet-header').should('have.class', 'hydrated');
cy.get('swisspost-internet-header', { timeout: 30000 }).should('have.class', 'hydrated');
});

it(`has nav item 'Briefe versenden' selected`, () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/internet-header/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"loader/"
],
"scripts": {
"dev": "stencil build --serve --port 9310 --watch --docs-readme",
"dev": "stencil build --serve --port 9310 --watch --docs-readme --dev",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This make it easier to use the browser debugger

"start": "stencil build --watch --docs-readme",
"build": "stencil build --docs-readme",
"clean": "rimraf www dist loader",
Expand All @@ -46,7 +46,6 @@
"body-scroll-lock": "4.0.0-beta.0",
"iframe-resizer": "4.3.9",
"jquery": "3.7.1",
"tabbable": "6.2.0",
"throttle-debounce": "5.0.0",
"url-polyfill": "1.1.12"
},
Expand All @@ -70,6 +69,7 @@
"cypress-each": "1.14.0",
"cypress-storybook": "0.5.1",
"eslint-plugin-react": "7.34.1",
"focus-trap": "7.5.4",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

focus-trap is from the same creator as tabbable and uses it inside

"globby": "14.0.1",
"jest": "29.7.0",
"jest-environment-jsdom": "29.7.0",
Expand Down
31 changes: 31 additions & 0 deletions packages/internet-header/src/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ export { StickynessOptions } from "./models/implementor.model";
export { Environment, ICustomConfig } from "./models/general.model";
export { IAvailableLanguage } from "./models/language.model";
export namespace Components {
/**
* Trap the focus inside a specific container.
* @param active activate or deactivate the focus trap
*/
interface FocusTrap {
"active": boolean;
}
interface PostKlpLoginWidget {
/**
* Override the logout-url provided by the portal config.
Expand Down Expand Up @@ -179,6 +186,16 @@ export interface SwisspostInternetHeaderCustomEvent<T> extends CustomEvent<T> {
target: HTMLSwisspostInternetHeaderElement;
}
declare global {
/**
* Trap the focus inside a specific container.
* @param active activate or deactivate the focus trap
*/
interface HTMLFocusTrapElement extends Components.FocusTrap, HTMLStencilElement {
}
var HTMLFocusTrapElement: {
prototype: HTMLFocusTrapElement;
new (): HTMLFocusTrapElement;
};
interface HTMLPostKlpLoginWidgetElement extends Components.PostKlpLoginWidget, HTMLStencilElement {
}
var HTMLPostKlpLoginWidgetElement: {
Expand Down Expand Up @@ -286,6 +303,7 @@ declare global {
new (): HTMLSwisspostInternetHeaderElement;
};
interface HTMLElementTagNameMap {
"focus-trap": HTMLFocusTrapElement;
"post-klp-login-widget": HTMLPostKlpLoginWidgetElement;
"post-language-switch": HTMLPostLanguageSwitchElement;
"post-logo": HTMLPostLogoElement;
Expand All @@ -299,6 +317,13 @@ declare global {
}
}
declare namespace LocalJSX {
/**
* Trap the focus inside a specific container.
* @param active activate or deactivate the focus trap
*/
interface FocusTrap {
"active"?: boolean;
}
interface PostKlpLoginWidget {
/**
* Override the logout-url provided by the portal config.
Expand Down Expand Up @@ -409,6 +434,7 @@ declare namespace LocalJSX {
"stickyness"?: StickynessOptions;
}
interface IntrinsicElements {
"focus-trap": FocusTrap;
"post-klp-login-widget": PostKlpLoginWidget;
"post-language-switch": PostLanguageSwitch;
"post-logo": PostLogo;
Expand All @@ -425,6 +451,11 @@ export { LocalJSX as JSX };
declare module "@stencil/core" {
export namespace JSX {
interface IntrinsicElements {
/**
* Trap the focus inside a specific container.
* @param active activate or deactivate the focus trap
*/
"focus-trap": LocalJSX.FocusTrap & JSXBase.HTMLAttributes<HTMLFocusTrapElement>;
"post-klp-login-widget": LocalJSX.PostKlpLoginWidget & JSXBase.HTMLAttributes<HTMLPostKlpLoginWidgetElement>;
"post-language-switch": LocalJSX.PostLanguageSwitch & JSXBase.HTMLAttributes<HTMLPostLanguageSwitchElement>;
"post-logo": LocalJSX.PostLogo & JSXBase.HTMLAttributes<HTMLPostLogoElement>;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { h } from '@stencil/core';
import { SvgIcon } from '../../../utils/svg-icon.component';
import { IBreadcrumbOverlay } from '../../../models/breadcrumbs.model';
import { FocusTrap } from './focus-trap.component';

/**
* Overlay implementation with focus trap according to
Expand All @@ -25,13 +24,8 @@ export const OverlayComponent = (props: {
ref={e => e !== undefined && props.overlayRef(e)}
>
<div class="container" role="dialog">
<FocusTrap>
<div
class="overlay-container"
tabindex="-1" /* For initial focus */
role="document"
onClick={e => e.stopPropagation()}
>
<focus-trap active={true}>
<div class="overlay-container" role="document" onClick={e => e.stopPropagation()}>
<button
class={`overlay-close btn-blank d-inline-flex align-items-center nav-link ${props.overlay.id}`}
onClick={() => props.onClick()}
Expand All @@ -46,7 +40,7 @@ export const OverlayComponent = (props: {
ref={e => e !== undefined && props.iFrameRef(e)}
></iframe>
</div>
</FocusTrap>
</focus-trap>
<div class="loader-wrapper">
<div class="loader"></div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@
position: relative;
background: white;
box-shadow: 0 0 40px rgba(0, 0, 0, 0.6);
visibility: hidden;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes the overlay not tabbable, I didn't see any differences without it.

transform: translateY(3rem);
opacity: 0;
transition: transform 500ms, opacity 500ms;
Expand Down Expand Up @@ -223,10 +222,10 @@ iframe {
height: 100%;
top: 0;
left: 0;
}

.loaded ~ & {
display: none;
}
.loaded .loader-wrapper {
display:none;
}

.hidden-control-breadcrumbs {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export class PostInternetBreadcrumbs {
[{ opacity: 1, visibility: 'visible', transform: 'translateY(0px)' }],
{ duration, fill: 'forwards' },
);
iFrame.parentElement?.classList.add('loaded');
iFrame.closest('[role=dialog]')?.classList.add('loaded');
this.loadedAnimation?.finished.then(() => {
iFrame.parentElement?.focus();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ Type: `Promise<void>`



## Dependencies

### Depends on

- [focus-trap](../shared)

### Graph
```mermaid
graph TD;
swisspost-internet-breadcrumbs --> focus-trap
style swisspost-internet-breadcrumbs fill:#f9f,stroke:#333,stroke-width:4px
```

----------------------------------------------

*Built with [StencilJS](https://stenciljs.com/)*
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ graph TD;
swisspost-internet-header --> post-main-navigation
swisspost-internet-header --> post-search
swisspost-internet-header --> post-klp-login-widget
post-search --> focus-trap
style swisspost-internet-header fill:#f9f,stroke:#333,stroke-width:4px
```

Expand Down
Loading
Loading