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: ensure components are disabled consistently #4108

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
fcbf241
fix: ensure components are disabled consistently
jcfranco Dec 22, 2021
fc0ae27
drop obsolete tile-select styles
jcfranco Jan 26, 2022
e4b40e2
remove stepper item overrides
jcfranco Jan 28, 2022
65840cc
prevent opacity stacking on slotted components
jcfranco Jan 28, 2022
739c4b5
enhance updateHostInteraction to restore tab index to 0 if host is ta…
jcfranco Jan 28, 2022
f709413
remove test reminders
jcfranco Jan 28, 2022
49190b3
remove redundant styles
jcfranco Jan 28, 2022
f5f84c9
ensure tile-select and tile-select-group can be disabled
jcfranco Jan 28, 2022
7295794
add interactive spec test
jcfranco Jan 29, 2022
4d9c73e
beef up disabled test helper
jcfranco Jan 29, 2022
11ca931
wire up tests
jcfranco Jan 29, 2022
4947d41
add disabled stories
jcfranco Jan 29, 2022
f378553
merge master
jcfranco Jan 31, 2022
7824109
merge master
jcfranco Feb 1, 2022
8250738
fix missed rename
jcfranco Feb 1, 2022
d40468f
fix typo
jcfranco Feb 1, 2022
aca508f
use element as focus target if shadow root has no active element
jcfranco Feb 1, 2022
2d86b4b
tweak updateHostInteraction + remove aria-disabled when false
jcfranco Feb 1, 2022
d0c62e6
update tests
jcfranco Feb 1, 2022
615ced7
wire up tile
jcfranco Feb 1, 2022
5f9d969
update tests
jcfranco Feb 2, 2022
e6bc681
update stepper item to keep host focusable
jcfranco Feb 2, 2022
48fbcce
tweak implementing components and add option to skip asserting focusa…
jcfranco Feb 2, 2022
56df805
beef up utils and doc
jcfranco Feb 2, 2022
0b9d8d5
wire up updateHostInteraction regardless of implementation
jcfranco Feb 2, 2022
a961bc7
update more components
jcfranco Feb 2, 2022
3421acb
tweak test helper
jcfranco Feb 2, 2022
8c44eef
fix rating test
jcfranco Feb 2, 2022
a16094f
merge master
jcfranco Feb 2, 2022
0ad2ed1
prevent page redirection
jcfranco Feb 2, 2022
bb398de
tweak helper
jcfranco Feb 2, 2022
6442cd8
only prevent default click behavior for internal <a> tags
jcfranco Feb 2, 2022
5a8a976
wire up missing components
jcfranco Feb 3, 2022
880e548
remove unused import
jcfranco Feb 4, 2022
f32244f
skip scrim for disabled lists
jcfranco Feb 4, 2022
8018e22
tweak helper
jcfranco Feb 4, 2022
b75ffb1
fix tests
jcfranco Feb 4, 2022
3495e5b
Merge branch 'master' into jcfranco/2655-ensure-consistent-disabled-b…
github-actions[bot] Feb 4, 2022
4c6a657
fix tests and tweak util
jcfranco Feb 4, 2022
503d811
tweak util
jcfranco Feb 4, 2022
62b2994
fix tests
jcfranco Feb 4, 2022
84c8ea5
tweaks
jcfranco Feb 4, 2022
9d285ba
fix test
jcfranco Feb 4, 2022
d477bcd
merge master
jcfranco Feb 4, 2022
0c87e8d
add frame delay before running assertions
jcfranco Feb 4, 2022
88d6a84
blur child element of disabled parent
jcfranco Feb 4, 2022
02cc057
merge master
jcfranco Feb 4, 2022
3931085
tweak helper
jcfranco Feb 5, 2022
548f378
fix inline-editable test
jcfranco Feb 5, 2022
3da8445
add missing interface to color picker
jcfranco Feb 5, 2022
477a494
restore link internal tabindex
jcfranco Feb 5, 2022
d372e64
tweak focus order reset logic
jcfranco Feb 5, 2022
915cc27
fix dropdown test
jcfranco Feb 5, 2022
6972039
fix list-item test
jcfranco Feb 5, 2022
0a717ef
fix color-picker test
jcfranco Feb 5, 2022
92cf2ec
remove unnecessary delays
jcfranco Feb 5, 2022
f10604a
tweak test util
jcfranco Feb 5, 2022
530534f
update rating test
jcfranco Feb 7, 2022
0df1b86
remove scrim overlay usage for disabled states
jcfranco Feb 7, 2022
08fa070
merge master
jcfranco Feb 8, 2022
3b76a3c
tidy up
jcfranco Feb 8, 2022
96fe6b3
add info to test
jcfranco Feb 8, 2022
25420d7
Merge branch 'master' into jcfranco/2655-ensure-consistent-disabled-b…
github-actions[bot] Feb 9, 2022
49c5f3c
hijack HTMLElement.click when component is disabled
jcfranco Feb 12, 2022
3582146
merge master
jcfranco Feb 13, 2022
42688eb
Merge branch 'master' into jcfranco/2655-ensure-consistent-disabled-b…
jcfranco Feb 14, 2022
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
15 changes: 15 additions & 0 deletions src/assets/styles/includes.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,18 @@
z-index: -1 !important;
}
}

// mixin to provide base disabled styles for interactive components
// additional styling can be passed via @content
@mixin disabled() {
:host([disabled]) {
@apply cursor-default opacity-disabled pointer-events-none select-none;
@content;

::slotted([calcite-hydrated][disabled]),
[calcite-hydrated][disabled] {
/* prevent opacity stacking */
@apply opacity-100;
}
}
}
26 changes: 3 additions & 23 deletions src/components/action/action.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { newE2EPage } from "@stencil/core/testing";
import { accessible, hidden, renders } from "../../tests/commonTests";
import { accessible, disabled, hidden, renders } from "../../tests/commonTests";
import { CSS } from "./resources";

describe("calcite-action", () => {
it("renders", async () => renders("calcite-action", { display: "flex" }));

it("honors hidden attribute", async () => hidden("calcite-action"));

it("can be disabled", () => disabled("calcite-action"));

it("should have visible text when text is enabled", async () => {
const page = await newE2EPage();
await page.setContent(`<calcite-action text="hello world" text-enabled></calcite-action>`);
Expand Down Expand Up @@ -99,14 +101,6 @@ describe("calcite-action", () => {
expect(button.getAttribute("aria-label")).toBe("hi");
});

it("should be disabled", async () => {
const page = await newE2EPage();
await page.setContent(`<calcite-action disabled></calcite-action>`);

const button = await page.find(`calcite-action >>> .${CSS.button}`);
expect(button).toHaveAttribute("disabled");
});

it("should have appearance=solid", async () => {
const page = await newE2EPage();
await page.setContent(`<calcite-action text="hello world"></calcite-action>`);
Expand All @@ -119,18 +113,4 @@ describe("calcite-action", () => {
await accessible(`<calcite-action text="hello world"></calcite-action>`);
await accessible(`<calcite-action text="hello world" disabled text-enabled></calcite-action>`);
});

it("should not emit click event when disabled", async () => {
const page = await newE2EPage();

await page.setContent(`<calcite-action text="hello world" disabled></calcite-action>`);

const action = await page.find("calcite-action");

const clickSpy = await action.spyOnEvent("click");

await action.click();

expect(clickSpy).toHaveReceivedEventTimes(0);
});
});
4 changes: 1 addition & 3 deletions src/components/action/action.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
@apply flex bg-transparent;
}

:host([disabled]) {
@apply pointer-events-none;
}
@include disabled();

.button {
@apply bg-foreground-1
Expand Down
7 changes: 6 additions & 1 deletion src/components/action/action.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { Alignment, Appearance, Scale } from "../interfaces";
import { CSS, TEXT } from "./resources";

import { createObserver } from "../../utils/observers";
import { InteractiveComponent, updateHostInteraction } from "../../utils/interactive";

/**
* @slot - A slot for adding a `calcite-icon`.
Expand All @@ -25,7 +26,7 @@ import { createObserver } from "../../utils/observers";
styleUrl: "action.scss",
shadow: true
})
export class Action {
export class Action implements InteractiveComponent {
// --------------------------------------------------------------------------
//
// Properties
Expand Down Expand Up @@ -133,6 +134,10 @@ export class Action {
this.mutationObserver?.disconnect();
}

componentDidRender(): void {
updateHostInteraction(this);
}

// --------------------------------------------------------------------------
//
// Methods
Expand Down
40 changes: 3 additions & 37 deletions src/components/block/block.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { newE2EPage } from "@stencil/core/testing";
import { CSS, SLOTS, TEXT } from "./resources";
import { accessible, defaults, hidden, renders, slots } from "../../tests/commonTests";
import { accessible, defaults, disabled, hidden, renders, slots } from "../../tests/commonTests";
import { html } from "../../tests/utils";

describe("calcite-block", () => {
Expand Down Expand Up @@ -43,42 +43,8 @@ describe("calcite-block", () => {
</calcite-block>
`));

it("can be disabled", async () => {
const page = await newE2EPage({
html: `
<calcite-block heading="heading" summary="summary" open collapsible>
<div class="content">content</div>
</calcite-block>
`
});

const content = await page.find(".content");
const clickSpy = await content.spyOnEvent("click");
await content.click();
expect(clickSpy).toHaveReceivedEventTimes(1);

const block = await page.find("calcite-block");
block.setProperty("disabled", true);
await page.waitForChanges();

// `tabindex=-1` on host removes children from the tab order
expect(block.getAttribute("tabindex")).toBe("-1");

await content.click();
expect(clickSpy).toHaveReceivedEventTimes(1);

const header = await page.find(`calcite-block >>> .${CSS.headerContainer}`);
const toggleSpy = await block.spyOnEvent("calciteBlockToggle");

await header.click();
await header.click();
expect(toggleSpy).toHaveReceivedEventTimes(0);

block.setAttribute("disabled", false);
await page.waitForChanges();

expect(block.getAttribute("tabindex")).toBeNull();
});
it("can be disabled", () =>
disabled(html`<calcite-block heading="heading" summary="summary" collapsible></calcite-block>`));

it("has a loading state", async () => {
const page = await newE2EPage({
Expand Down
13 changes: 2 additions & 11 deletions src/components/block/block.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
flex-basis: auto;
}

@include disabled();

@import "../../assets/styles/header";

.header {
Expand Down Expand Up @@ -161,14 +163,3 @@ calcite-action-menu {
@apply text-color-1;
}
}

:host([disabled]) {
pointer-events: none;
user-select: none;

@apply pointer-events-none select-none;

.header-container {
@apply opacity-50;
}
}
6 changes: 6 additions & 0 deletions src/components/block/block.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,9 @@ export const withHeaderControl = (): string =>

export const withIconAndHeader = (): string =>
create("calcite-block", createBlockAttributes({ exceptions: ["open", "collapsible"] }), `<div slot="icon">✅</div>`);

export const disabled = (): string => html`<calcite-block heading="heading" summary="summary" open collapsible>
<calcite-block-section text="Nature" open>
<img alt="demo" src="${placeholderImage({ width: 320, height: 240 })}" />
</calcite-block-section>
</calcite-block>`;
18 changes: 14 additions & 4 deletions src/components/block/block.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
connectConditionalSlotComponent,
disconnectConditionalSlotComponent
} from "../../utils/conditionalSlot";
import { InteractiveComponent, updateHostInteraction } from "../../utils/interactive";

/**
* @slot - A slot for adding content to the block.
Expand All @@ -20,7 +21,7 @@ import {
styleUrl: "block.scss",
shadow: true
})
export class Block implements ConditionalSlotComponent {
export class Block implements ConditionalSlotComponent, InteractiveComponent {
// --------------------------------------------------------------------------
//
// Properties
Expand Down Expand Up @@ -92,6 +93,16 @@ export class Block implements ConditionalSlotComponent {
*/
@Prop() summary: string;

//--------------------------------------------------------------------------
//
// Lifecycle
//
//--------------------------------------------------------------------------

componentDidRender(): void {
updateHostInteraction(this);
}

// --------------------------------------------------------------------------
//
// Private Properties
Expand Down Expand Up @@ -190,8 +201,7 @@ export class Block implements ConditionalSlotComponent {
}

render(): VNode {
const { collapsible, disabled, el, intlCollapse, intlExpand, loading, open, intlLoading } =
this;
const { collapsible, el, intlCollapse, intlExpand, loading, open, intlLoading } = this;

const toggleLabel = open ? intlCollapse || TEXT.collapse : intlExpand || TEXT.expand;

Expand Down Expand Up @@ -246,7 +256,7 @@ export class Block implements ConditionalSlotComponent {
);

return (
<Host tabIndex={disabled ? -1 : null}>
<Host>
<article
aria-busy={loading.toString()}
class={{
Expand Down
4 changes: 3 additions & 1 deletion src/components/button/button.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { E2EElement, newE2EPage } from "@stencil/core/testing";
import { accessible, HYDRATED_ATTR, labelable } from "../../tests/commonTests";
import { accessible, disabled, HYDRATED_ATTR, labelable } from "../../tests/commonTests";
import { CSS } from "./resources";
import { html, GlobalTestProps } from "../../tests/utils";

Expand Down Expand Up @@ -44,6 +44,8 @@ describe("calcite-button", () => {

it("is labelable", async () => labelable("calcite-button"));

it("can be disabled", () => disabled("calcite-button"));

it("should update childElType when href changes", async () => {
const page = await newE2EPage({ html: `<calcite-button>Continue</calcite-button>` });
const link = await page.find("calcite-button");
Expand Down
6 changes: 3 additions & 3 deletions src/components/button/button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@
line-height: inherit;
}

// disabled styles
:host([loading]),
:host([disabled]) {
@include disabled();

:host([loading]) {
@apply pointer-events-none;
button,
a {
Expand Down
4 changes: 3 additions & 1 deletion src/components/button/button.stories.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { text, select } from "@storybook/addon-knobs";
import { iconNames, boolean } from "../../../.storybook/helpers";
import { themesDarkDefault } from "../../../.storybook/utils";
import { html } from "../../tests/utils";
import { html, placeholderImage } from "../../tests/utils";
import readme from "./readme.md";

export default {
Expand Down Expand Up @@ -175,3 +175,5 @@ export const RTL = (): string => html`
${text("text", "button text here")}
</calcite-button>
`;

export const disabled = (): string => html`<calcite-button disabled>disabled</calcite-button>`;
7 changes: 6 additions & 1 deletion src/components/button/button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { ButtonAlignment, ButtonAppearance, ButtonColor } from "./interfaces";
import { FlipContext, Scale, Width } from "../interfaces";
import { LabelableComponent, connectLabel, disconnectLabel, getLabelText } from "../../utils/label";
import { createObserver } from "../../utils/observers";
import { InteractiveComponent, updateHostInteraction } from "../../utils/interactive";

/** Passing a 'href' will render an anchor link, instead of a button. Role will be set to link, or button, depending on this. */
/** It is the consumers responsibility to add aria information, rel, target, for links, and any button attributes for form submission */
Expand All @@ -16,7 +17,7 @@ import { createObserver } from "../../utils/observers";
styleUrl: "button.scss",
shadow: true
})
export class Button implements LabelableComponent {
export class Button implements LabelableComponent, InteractiveComponent {
//--------------------------------------------------------------------------
//
// Element
Expand Down Expand Up @@ -146,6 +147,10 @@ export class Button implements LabelableComponent {
}
}

componentDidRender(): void {
updateHostInteraction(this);
}

render(): VNode {
const Tag = this.childElType;
const loaderNode = this.hasLoader ? (
Expand Down
19 changes: 3 additions & 16 deletions src/components/checkbox/checkbox.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { newE2EPage } from "@stencil/core/testing";
import { accessible, focusable, formAssociated, HYDRATED_ATTR, labelable } from "../../tests/commonTests";
import { accessible, disabled, focusable, formAssociated, HYDRATED_ATTR, labelable } from "../../tests/commonTests";

describe("calcite-checkbox", () => {
it("is accessible", async () =>
Expand All @@ -15,6 +15,8 @@ describe("calcite-checkbox", () => {

it("is form-associated", async () => formAssociated("calcite-checkbox", { testValue: true }));

it("can be disabled", () => disabled("calcite-checkbox"));

it("renders with correct default attributes", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-checkbox></calcite-checkbox>");
Expand Down Expand Up @@ -70,21 +72,6 @@ describe("calcite-checkbox", () => {
expect(spy).toHaveReceivedEventTimes(0);
});

it("does not toggle when clicked if disabled", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-checkbox disabled></calcite-checkbox>");

const calciteCheckbox = await page.find("calcite-checkbox");

expect(calciteCheckbox).not.toHaveAttribute("checked");

await calciteCheckbox.click();

await page.waitForChanges();

expect(calciteCheckbox).not.toHaveAttribute("checked");
});

it("removes the indeterminate attribute when clicked", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-checkbox indeterminate></calcite-checkbox>");
Expand Down
4 changes: 1 addition & 3 deletions src/components/checkbox/checkbox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@
@include focus-box-shadow(inset 0 0 0 1px var(--calcite-ui-brand));
}
}
:host([disabled]) {
@apply cursor-default opacity-disabled pointer-events-none;
}

@include disabled();
@include hidden-form-input();
2 changes: 2 additions & 0 deletions src/components/checkbox/checkbox.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,5 @@ export const RTL = (): string => html`
${text("label", "Checkbox")}
</calcite-label>
`;

export const disabled = (): string => html`<calcite-checkbox checked disabled></calcite-checkbox>`;
Loading