-
Notifications
You must be signed in to change notification settings - Fork 56
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
Close menu when focus leaves #1367
Conversation
Signed-off-by: Christopher Ng <[email protected]>
6ecfbce
to
f6db640
Compare
/backport to stable25 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels weird to have to do this manually 😬
:deep(.button-vue) { | ||
&:focus { | ||
outline: 2px solid var(--color-main-text) !important; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth raising an upstream issue in the vue library? That should basically affect every place this is done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on https://github.com/nextcloud/nextcloud-vue/blob/ade7ba30f759700b3a0b28dca227009ef8157153/src/components/NcButton/NcButton.vue#L426-L429 would say this should only be done in specific cases like these where it is necessary
When programmatically setting focus on an element i.e.
element.focus()
only:focus
styles are applied but not:focus-visible
and the API which supports this is still experimental (see references below)So we add the style declaration https://github.com/nextcloud/nextcloud-vue/blob/10fbb7829803731a26ebae2f0ce0be40d26901a1/src/components/NcButton/NcButton.vue#L501 to the
:focus
pseudo-class to resolve thisReferences
:focus-visible
behavior. whatwg/html#7830