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.

Sign up for GitHub

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

Jump to bottom

fix(action-bar): include focus-visible polyfilling #4273

Merged
merged 2 commits into from
May 10, 2024
Merged

fix(action-bar): include focus-visible polyfilling #4273

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c5171e
Fixed sp-action-bar to handle focus-visible
inasirhussain Apr 16, 2024
ca4b465
chore: accept code review
Westbrook May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/action-bar/src/ActionBar.ts
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@ import {
CSSResultArray,
html,
SpectrumElement,
SizedMixin,
Westbrook marked this conversation as resolved.
Show resolved Hide resolved
TemplateResult,
} from '@spectrum-web-components/base';
import { property } from '@spectrum-web-components/base/src/decorators.js';
Expand All @@ -23,13 +24,16 @@ import '@spectrum-web-components/button/sp-close-button.js';
import '@spectrum-web-components/field-label/sp-field-label.js';
import actionBarStyles from './action-bar.css.js';
import { ifDefined } from '@spectrum-web-components/base/src/directives.js';
import { FocusVisiblePolyfillMixin } from "@spectrum-web-components/shared/src/focus-visible.js";
export const actionBarVariants = ['sticky', 'fixed'];

/**
* @element sp-action-bar
* @slot - Content to display with the Action Bar
*/
export class ActionBar extends SpectrumElement {
export class ActionBar extends SizedMixin(
FocusVisiblePolyfillMixin(SpectrumElement)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems reasonable for the close button. Would be great if UXP prioritized adding this, as I'm sure there are lots of little places where UI like this falls through the cracks and keeping the polyfill code for something that has been fixed in browser for so long is questionable on our end.

) {
Westbrook marked this conversation as resolved.
Show resolved Hide resolved
public static override get styles(): CSSResultArray {
return [actionBarStyles];
}
Expand Down