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

fix(action-bar): no longer delegates focus when clicked on non-focusable region #7310

Merged
merged 8 commits into from
Jul 12, 2023
Merged

fix(action-bar): no longer delegates focus when clicked on non-focusable region #7310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6639b33
fix(action-bar): no longer delegates focus when clicked on non-focusa…
anveshmekala Jul 11, 2023
092c3c6
Merge branch 'main' into anveshmekala/6462-fix-action-bar-focus-on-em…
anveshmekala Jul 12, 2023
0984ec5
add e2e test
anveshmekala Jul 12, 2023
e2dfa2b
remove commented code
anveshmekala Jul 12, 2023
17234e6
apply ms-auto only on horizontal layout
anveshmekala Jul 12, 2023
b9aae87
add margin-block for vertical layout
anveshmekala Jul 12, 2023
82996dc
feedback changes
anveshmekala Jul 12, 2023
d3e8b49
revert bottomActions ref el
anveshmekala Jul 12, 2023
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
16 changes: 16 additions & 0 deletions packages/calcite-components/src/components/action-bar/action-bar.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { html } from "../../../support/formatting";
import { accessible, defaults, focusable, hidden, reflects, renders, slots, t9n } from "../../tests/commonTests";
import { CSS, SLOTS } from "./resources";
import { overflowActionsDebounceInMs } from "./utils";
import { getFocusedElementProp } from "../../tests/utils";

describe("calcite-action-bar", () => {
describe("renders", () => {
Expand Down Expand Up @@ -247,6 +248,21 @@ describe("calcite-action-bar", () => {
focusTargetSelector: "calcite-action"
}
);

it("should not focus any action element when clicked on non-focusable region", async () => {
const page = await newE2EPage();
await page.setContent(html` <calcite-action-bar layout="horizontal" style="width: 100%">
<calcite-action-group> <calcite-action text="Add" icon="plus"> </calcite-action> </calcite-action-group
></calcite-action-bar>`);

const actionBarElRect = await page.evaluate(() => {
const actionBarEl = document.querySelector("calcite-action-bar");
return actionBarEl.getBoundingClientRect().toJSON();
});

await page.mouse.click(actionBarElRect.right / 2, actionBarElRect.y + actionBarElRect.bottom / 2);
expect(await getFocusedElementProp(page, "tagName", { shadow: true })).not.toBe("CALCITE-ACTION");
});
});

describe("slots", () => {
Expand Down
8 changes: 7 additions & 1 deletion packages/calcite-components/src/components/action-bar/action-bar.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,16 @@

:host([layout="vertical"]) {
@apply flex-col;
.action-group--bottom {
margin-block-start: auto;
}
}

:host([layout="horizontal"]) {
@apply flex-row;
.action-group--bottom {
@apply ms-auto;
}
}

:host([layout="vertical"][overflow-actions-disabled]) {
Expand Down Expand Up @@ -53,5 +59,5 @@
}

.action-group--bottom {
@apply flex-grow justify-end;
Copy link
Member

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Contributor Author

@anveshmekala anveshmekala Jul 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

With flex-grow, the calcite-action-group is filling up the space in full width mode. Any click on the space filled is delegatingFocus in calcite-action-group. To avoid this, the empty space will be filled by calcite-action-bar and delegateFocus will be removed from calcite-action-bar.

@apply justify-end;
}
18 changes: 11 additions & 7 deletions packages/calcite-components/src/components/action-bar/action-bar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
connectConditionalSlotComponent,
disconnectConditionalSlotComponent
} from "../../utils/conditionalSlot";
import { getSlotted, slotChangeGetAssignedElements } from "../../utils/dom";
import { focusFirstTabbable, getSlotted, slotChangeGetAssignedElements } from "../../utils/dom";
import {
componentFocusable,
LoadableComponent,
Expand Down Expand Up @@ -53,14 +53,20 @@ import {
@Component({
tag: "calcite-action-bar",
styleUrl: "action-bar.scss",
shadow: {
delegatesFocus: true
},
shadow: true,
assetsDirs: ["assets"]
})
export class ActionBar
implements ConditionalSlotComponent, LoadableComponent, LocalizedComponent, T9nComponent
{
//--------------------------------------------------------------------------
//
// Element
//
//--------------------------------------------------------------------------

@Element() el: HTMLCalciteActionBarElement;

// --------------------------------------------------------------------------
//
// Properties
Expand Down Expand Up @@ -157,8 +163,6 @@ export class ActionBar
//
// --------------------------------------------------------------------------

@Element() el: HTMLCalciteActionBarElement;

mutationObserver = createObserver("mutation", () => {
const { el, expanded } = this;
toggleChildActionText({ el, expanded });
Expand Down Expand Up @@ -245,7 +249,7 @@ export class ActionBar
async setFocus(): Promise<void> {
await componentFocusable(this);

this.el?.focus();
focusFirstTabbable(this.el);
}

// --------------------------------------------------------------------------
Expand Down