-
Notifications
You must be signed in to change notification settings - Fork 21
feat(components): enable open animation on <post tooltip>
#6821
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
base: main
Are you sure you want to change the base?
Changes from all commits
5736d8f
137d4fc
12c1ebe
49b054d
7d49eb2
acc0a7e
69c496a
7eb2f39
c82051b
013d70d
e39b70d
698b33b
799d860
aec4869
eab4e5f
46ba2f7
947679b
f5dac59
6f2e62f
7328a02
ad74696
46d9592
a7d1ca4
87ab0af
3e79ea0
00b54f1
1fad5bc
53c41f5
afe29ae
c855b0b
68fd70c
64d07bb
3feb44f
5370cf2
17ec978
29bac1c
1ce729a
c2857ce
877f53a
010d01d
31c0035
2251b43
6b991e2
b401d79
a522200
9e4859c
5b47f20
cc818b3
a4d874c
526bbbd
e984179
73e82c4
60af7f7
15a1115
c6e4130
63a5391
0e21e37
5333219
f6eacb9
bef878a
836306e
7880fb5
93da67d
c19a101
64e2f00
de27e1d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@swisspost/design-system-components': minor | ||
| --- | ||
|
|
||
| Enabled the open animation for the `<post-tooltip>` component. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@swisspost/design-system-components': minor | ||
| --- | ||
|
|
||
| Enabled pop-in animations for the concatenated `post-breadcrumbs`, the `post-language-menu`, and the header's user menu components. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,8 @@ export class PostPopover { | |
| */ | ||
| @Method() | ||
| async show(target: HTMLElement) { | ||
| this.popoverRef.show(target); | ||
| await this.popoverRef.show(target); | ||
| this.focusFirstEl(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -75,10 +76,13 @@ export class PostPopover { | |
| @Method() | ||
| async toggle(target: HTMLElement, force?: boolean) { | ||
| await this.popoverRef.toggle(target, force); | ||
| this.focusFirstEl(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we avoid calling this function when we toggle to false?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I 'm not sure. I had already thought about it, but currently post-menu toggle() calls it like this - without the force:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, that was not my intention, but it would be good if it does the exact same thing. I just had a look into your PR and I think we can do the following to optimize this functionality: async toggle(target: HTMLElement, force?: boolean) {
// you can expect the popoverRef.toggle function to return the current state (after toggling), no matter if we call it with a `force` parameter or not ;)
const isOpen = await this.popoverRef.toggle(target, force);
if (isOpen) this.focusFirstEl();
}This is only a micro improvement reguarding performance, but it could save us from unwanted side-effects. |
||
| } | ||
|
|
||
| private focusFirstEl() { | ||
| const focusableChildren = getDeepFocusableChildren(this.host); | ||
|
|
||
| // find first focusable element | ||
| // Find first focusable element | ||
| const firstFocusable = focusableChildren[0]; | ||
|
|
||
| if (firstFocusable) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.