From d81a03a2bc238dba836f5230c1d0691ec630676a Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Thu, 8 Jun 2023 14:17:46 -0700 Subject: [PATCH 1/3] fix(alert): Sets autoCloseDuration to "medium" by default. #6363 --- .../src/components/alert/alert.e2e.ts | 11 ++++++++++- .../calcite-components/src/components/alert/alert.tsx | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/calcite-components/src/components/alert/alert.e2e.ts b/packages/calcite-components/src/components/alert/alert.e2e.ts index 1c266cf3a89..aad265b893e 100644 --- a/packages/calcite-components/src/components/alert/alert.e2e.ts +++ b/packages/calcite-components/src/components/alert/alert.e2e.ts @@ -1,9 +1,18 @@ import { E2EElement, E2EPage, newE2EPage } from "@stencil/core/testing"; import { html } from "../../../support/formatting"; -import { accessible, hidden, HYDRATED_ATTR, renders, t9n } from "../../tests/commonTests"; +import { accessible, defaults, hidden, HYDRATED_ATTR, renders, t9n } from "../../tests/commonTests"; import { getElementXY } from "../../tests/utils"; import { CSS, DURATIONS } from "./resources"; +describe("defaults", () => { + defaults("calcite-alert", [ + { + propertyName: "autoCloseDuration", + defaultValue: "medium" + } + ]); +}); + describe("calcite-alert", () => { const alertContent = `
Title Text
diff --git a/packages/calcite-components/src/components/alert/alert.tsx b/packages/calcite-components/src/components/alert/alert.tsx index d748079d228..a5c84406150 100644 --- a/packages/calcite-components/src/components/alert/alert.tsx +++ b/packages/calcite-components/src/components/alert/alert.tsx @@ -100,7 +100,7 @@ export class Alert implements OpenCloseComponent, LoadableComponent, T9nComponen @Prop({ reflect: true }) autoClose = false; /** Specifies the duration before the component automatically closes (only use with `autoClose`). */ - @Prop({ reflect: true }) autoCloseDuration: AlertDuration = this.autoClose ? "medium" : null; + @Prop({ reflect: true }) autoCloseDuration: AlertDuration = "medium"; /** Specifies the kind of the component (will apply to top border and icon). */ @Prop({ reflect: true }) kind: Extract< From cb0acb77e438c404a8705f98bf939f43f5b9aa8b Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Fri, 9 Jun 2023 11:42:47 -0700 Subject: [PATCH 2/3] update conventions --- .../calcite-components/conventions/README.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/calcite-components/conventions/README.md b/packages/calcite-components/conventions/README.md index f8edf90bc90..bd9206a83a5 100644 --- a/packages/calcite-components/conventions/README.md +++ b/packages/calcite-components/conventions/README.md @@ -153,6 +153,22 @@ It is recommended to reflect properties that fit the following criteria: Doing so will give developers more flexibility when querying the DOM. This is important in framework environments where we can't safely assume components will have their attributes set vs properties. +### Property defaults + +Properties should have static default values. There should be no conditional logic when defining the default value of a property. + +#### Incorrect example + +```ts +@Prop({ reflect: true }) autoCloseDuration: AlertDuration = this.autoClose ? "medium" : null; +``` + +#### Correct example + +```ts +@Prop({ reflect: true }) autoCloseDuration: AlertDuration = "medium"; +``` + ### `ref` usage Due to a [bug in Stencil](https://github.com/ionic-team/stencil/issues/4074), `ref` should be set as the last property in JSX to ensure the node's attributes/properties are up to date. From a74b0e0ea102947c9e4a2c00b4e74531dc71346b Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Thu, 15 Jun 2023 16:41:54 -0700 Subject: [PATCH 3/3] revert convention --- .../calcite-components/conventions/README.md | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/calcite-components/conventions/README.md b/packages/calcite-components/conventions/README.md index bd9206a83a5..f8edf90bc90 100644 --- a/packages/calcite-components/conventions/README.md +++ b/packages/calcite-components/conventions/README.md @@ -153,22 +153,6 @@ It is recommended to reflect properties that fit the following criteria: Doing so will give developers more flexibility when querying the DOM. This is important in framework environments where we can't safely assume components will have their attributes set vs properties. -### Property defaults - -Properties should have static default values. There should be no conditional logic when defining the default value of a property. - -#### Incorrect example - -```ts -@Prop({ reflect: true }) autoCloseDuration: AlertDuration = this.autoClose ? "medium" : null; -``` - -#### Correct example - -```ts -@Prop({ reflect: true }) autoCloseDuration: AlertDuration = "medium"; -``` - ### `ref` usage Due to a [bug in Stencil](https://github.com/ionic-team/stencil/issues/4074), `ref` should be set as the last property in JSX to ensure the node's attributes/properties are up to date.