From 31f8a3e11e7d85e45327e4f204bf1229af5d3d11 Mon Sep 17 00:00:00 2001 From: Chris Holt <13071055+chrisdholt@users.noreply.github.com> Date: Mon, 10 Jun 2024 17:30:04 -0700 Subject: [PATCH 1/7] refactor progress bar to use ElementInternals --- ...-6d07cdaf-1af6-4d87-9232-7dd09295ec30.json | 7 + packages/web-components/docs/api-report.md | 31 ++- packages/web-components/src/index.ts | 1 - .../src/progress-bar/base-progress.ts | 71 ------- .../web-components/src/progress-bar/index.ts | 1 - .../src/progress-bar/progress-bar.options.ts | 9 - .../src/progress-bar/progress-bar.spec.ts | 32 +--- .../src/progress-bar/progress-bar.stories.ts | 17 +- .../src/progress-bar/progress-bar.styles.ts | 179 +++++------------- .../src/progress-bar/progress-bar.template.ts | 39 +--- .../src/progress-bar/progress-bar.ts | 103 +++++++++- 11 files changed, 194 insertions(+), 296 deletions(-) create mode 100644 change/@fluentui-web-components-6d07cdaf-1af6-4d87-9232-7dd09295ec30.json delete mode 100644 packages/web-components/src/progress-bar/base-progress.ts diff --git a/change/@fluentui-web-components-6d07cdaf-1af6-4d87-9232-7dd09295ec30.json b/change/@fluentui-web-components-6d07cdaf-1af6-4d87-9232-7dd09295ec30.json new file mode 100644 index 0000000000000..b91ffbbea5526 --- /dev/null +++ b/change/@fluentui-web-components-6d07cdaf-1af6-4d87-9232-7dd09295ec30.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "refactor progress bar to use ElementInternals", + "packageName": "@fluentui/web-components", + "email": "13071055+chrisdholt@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/web-components/docs/api-report.md b/packages/web-components/docs/api-report.md index a2c2435cb5197..d366c71a38266 100644 --- a/packages/web-components/docs/api-report.md +++ b/packages/web-components/docs/api-report.md @@ -1888,7 +1888,6 @@ export function display(displayValue: CSSDisplayPropertyValue): string; // // @public export class Divider extends FASTElement { - constructor(); // (undocumented) alignContent?: DividerAlignContent; // (undocumented) @@ -2437,13 +2436,31 @@ export const MenuStyles: ElementStyles; // @public (undocumented) export const MenuTemplate: ElementViewTemplate; -// Warning: (ae-forgotten-export) The symbol "BaseProgress" needs to be exported by the entry point index.d.ts -// // @public -export class ProgressBar extends BaseProgress { +export class ProgressBar extends FASTElement { + // @internal + constructor(); + // @internal (undocumented) + connectedCallback(): void; + // @internal + elementInternals: ElementInternals; + // @internal + max?: number; + // (undocumented) + protected maxChanged(): void; + // @internal + min?: number; + // (undocumented) + protected minChanged(): void; + // @internal + percentComplete: number; shape?: ProgressBarShape; thickness?: ProgressBarThickness; validationState: ProgressBarValidationState | null; + // @internal + value?: number | null; + // (undocumented) + protected valueChanged(): void; } // @public @@ -2485,12 +2502,6 @@ export const ProgressBarValidationState: { // @public export type ProgressBarValidationState = ValuesOf; -// @public -export type ProgressOptions = { - indeterminateIndicator1?: StaticallyComposableHTML; - indeterminateIndicator2?: StaticallyComposableHTML; -}; - // Warning: (ae-forgotten-export) The symbol "FormAssociatedRadio" needs to be exported by the entry point index.d.ts // // @public diff --git a/packages/web-components/src/index.ts b/packages/web-components/src/index.ts index 419694b75c212..7e7e95b5a8775 100644 --- a/packages/web-components/src/index.ts +++ b/packages/web-components/src/index.ts @@ -140,7 +140,6 @@ export { ProgressBarThickness, ProgressBarValidationState, } from './progress-bar/index.js'; -export type { ProgressOptions } from './progress-bar/index.js'; export { RadioGroup, RadioGroupDefinition, diff --git a/packages/web-components/src/progress-bar/base-progress.ts b/packages/web-components/src/progress-bar/base-progress.ts deleted file mode 100644 index aaf8411121521..0000000000000 --- a/packages/web-components/src/progress-bar/base-progress.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { attr, FASTElement, nullableNumberConverter, observable } from '@microsoft/fast-element'; - -/** - * A base class for progress components. - * @public - */ -export class BaseProgress extends FASTElement { - /** - * The value of the progress - * @public - * @remarks - * HTML Attribute: value - */ - @attr({ converter: nullableNumberConverter }) - public value!: number | null; - protected valueChanged(): void { - this.updatePercentComplete(); - } - - /** - * The minimum value - * @public - * @remarks - * HTML Attribute: min - */ - @attr({ converter: nullableNumberConverter }) - public min!: number; - protected minChanged(): void { - if (this.$fastController.isConnected) { - this.updatePercentComplete(); - } - } - - /** - * The maximum value - * @public - * @remarks - * HTML Attribute: max - */ - @attr({ converter: nullableNumberConverter }) - public max!: number; - protected maxChanged(): void { - if (this.$fastController.isConnected) { - this.updatePercentComplete(); - } - } - - /** - * Indicates progress in % - * @internal - */ - @observable - public percentComplete: number = 0; - - /** - * @internal - */ - public connectedCallback(): void { - super.connectedCallback(); - this.updatePercentComplete(); - } - - private updatePercentComplete(): void { - const min: number = typeof this.min === 'number' ? this.min : 0; - const max: number = typeof this.max === 'number' ? this.max : 100; - const value: number = typeof this.value === 'number' ? this.value : 0; - const range: number = max - min; - - this.percentComplete = range === 0 ? 0 : Math.fround(((value - min) / range) * 100); - } -} diff --git a/packages/web-components/src/progress-bar/index.ts b/packages/web-components/src/progress-bar/index.ts index 3cef34b54fff4..c28ec85507de3 100644 --- a/packages/web-components/src/progress-bar/index.ts +++ b/packages/web-components/src/progress-bar/index.ts @@ -1,6 +1,5 @@ export { definition as ProgressBarDefinition } from './progress-bar.definition.js'; export { ProgressBar } from './progress-bar.js'; export { ProgressBarShape, ProgressBarThickness, ProgressBarValidationState } from './progress-bar.options.js'; -export type { ProgressOptions } from './progress-bar.options.js'; export { styles as ProgressBarStyles } from './progress-bar.styles.js'; export { template as ProgressBarTemplate } from './progress-bar.template.js'; diff --git a/packages/web-components/src/progress-bar/progress-bar.options.ts b/packages/web-components/src/progress-bar/progress-bar.options.ts index 96b4d74b635cd..9b1e5a27e13c9 100644 --- a/packages/web-components/src/progress-bar/progress-bar.options.ts +++ b/packages/web-components/src/progress-bar/progress-bar.options.ts @@ -46,12 +46,3 @@ export const ProgressBarValidationState = { * @public */ export type ProgressBarValidationState = ValuesOf; - -/** - * Progress configuration options - * @public - */ -export type ProgressOptions = { - indeterminateIndicator1?: StaticallyComposableHTML; - indeterminateIndicator2?: StaticallyComposableHTML; -}; diff --git a/packages/web-components/src/progress-bar/progress-bar.spec.ts b/packages/web-components/src/progress-bar/progress-bar.spec.ts index 119ce516ef0ae..5db519b56836e 100644 --- a/packages/web-components/src/progress-bar/progress-bar.spec.ts +++ b/packages/web-components/src/progress-bar/progress-bar.spec.ts @@ -24,7 +24,7 @@ test.describe('Progress Bar', () => { // Foundation tests test('should include a role of progressbar', async () => { - await expect(element).toHaveAttribute('role', 'progressbar'); + await expect(element).toHaveJSProperty('elementInternals.role', 'progressbar'); }); test('should set the `aria-valuenow` attribute with the `value` property when provided', async () => { @@ -34,7 +34,7 @@ test.describe('Progress Bar', () => { `; }); - await expect(element).toHaveAttribute('aria-valuenow', '50'); + await expect(element).toHaveJSProperty('elementInternals.ariaValueNow', '50'); }); test('should set the `aria-valuemin` attribute with the `min` property when provided', async () => { @@ -44,7 +44,7 @@ test.describe('Progress Bar', () => { `; }); - await expect(element).toHaveAttribute('aria-valuemin', '50'); + await expect(element).toHaveJSProperty('ariaValueMin', '50'); }); test('should set the `aria-valuemax` attribute with the `max` property when provided', async () => { @@ -54,31 +54,7 @@ test.describe('Progress Bar', () => { `; }); - await expect(element).toHaveAttribute('aria-valuemax', '50'); - }); - - test('should render an element with a `determinate` slot when a value is provided', async () => { - await root.evaluate(node => { - node.innerHTML = /* html */ ` - - `; - }); - - const progress = element.locator('.progress'); - - await expect(progress).toHaveAttribute('slot', 'determinate'); - }); - - test('should render an element with an `indeterminate` slot when no value is provided', async () => { - await root.evaluate(node => { - node.innerHTML = /* html */ ` - - `; - }); - - const progress = element.locator('.progress'); - - await expect(progress).toHaveAttribute('slot', 'indeterminate'); + await expect(element).toHaveJSProperty('ariaValueMax', '50'); }); test('should return the `percentComplete` property as a value between 0 and 100 when `min` and `max` are unset', async () => { diff --git a/packages/web-components/src/progress-bar/progress-bar.stories.ts b/packages/web-components/src/progress-bar/progress-bar.stories.ts index 77e2347757131..762e7daa4e6ff 100644 --- a/packages/web-components/src/progress-bar/progress-bar.stories.ts +++ b/packages/web-components/src/progress-bar/progress-bar.stories.ts @@ -13,8 +13,7 @@ const storyTemplate = html` thickness=${x => x.thickness} shape=${x => x.shape} max=${x => x.max} - aria-valuemax=${x => x.max} - aria-valuenow=${x => x.value} + min=${x => x.min} value=${x => x.value} validation-state=${x => x.validationState} > @@ -72,26 +71,26 @@ export const Max = renderComponent(html`

3 of 10 - +

3 o 5 - +

`); export const Value = renderComponent(html`
- 0 + 0 25 - + 50 - + 75 - + 100 - +
`); diff --git a/packages/web-components/src/progress-bar/progress-bar.styles.ts b/packages/web-components/src/progress-bar/progress-bar.styles.ts index 3898d93b1b986..ab3295c6c4d6a 100644 --- a/packages/web-components/src/progress-bar/progress-bar.styles.ts +++ b/packages/web-components/src/progress-bar/progress-bar.styles.ts @@ -2,184 +2,103 @@ import { css } from '@microsoft/fast-element'; import { display, forcedColorsStylesheetBehavior } from '../utils/index.js'; import { borderRadiusMedium, - colorBrandBackground2, + borderRadiusNone, colorCompoundBrandBackground, colorNeutralBackground6, - colorPaletteDarkOrangeBackground2, colorPaletteDarkOrangeBackground3, - colorPaletteGreenBackground2, colorPaletteGreenBackground3, - colorPaletteRedBackground2, colorPaletteRedBackground3, + colorTransparentBackground, } from '../theme/design-tokens.js'; /** ProgressBar styles * @public */ export const styles = css` - ${display('flex')} + ${display('block')} :host { - align-items: center; + width: 100%; height: 2px; overflow-x: hidden; + background-color: ${colorNeutralBackground6}; border-radius: ${borderRadiusMedium}; contain: content; } - :host([thickness='large']), - :host([thickness='large']) .progress, - :host([thickness='large']) .determinate { + :host([thickness='large']) { height: 4px; } - :host([shape='square']), - :host([shape='square']) .progress, - :host([shape='square']) .determinate { - border-radius: 0; + :host([shape='square']) { + border-radius: ${borderRadiusNone}; } - :host([validation-state='error']) .determinate { - background-color: ${colorPaletteRedBackground3}; - } - - :host([validation-state='error']) .indeterminate-indicator-1, - :host([validation-state='error']) .indeterminate-indicator-2 { - background: linear-gradient( - to right, - ${colorPaletteRedBackground2} 0%, - ${colorPaletteRedBackground3} 50%, - ${colorPaletteRedBackground2} - ); - } - - :host([validation-state='warning']) .determinate { - background-color: ${colorPaletteDarkOrangeBackground3}; - } - - :host([validation-state='warning']) .indeterminate-indicator-1, - :host([validation-state='warning']) .indeterminate-indicator-2 { - background: linear-gradient( - to right, - ${colorPaletteDarkOrangeBackground2} 0%, - ${colorPaletteDarkOrangeBackground3} 50%, - ${colorPaletteDarkOrangeBackground2} - ); + .indicator { + background-color: ${colorCompoundBrandBackground}; + border-radius: inherit; + height: 100%; } - :host([validation-state='success']) .determinate { - background-color: ${colorPaletteGreenBackground3}; + :host([value]) .indicator { + transition: all 0.2s ease-in-out; } - :host([validation-state='success']) .indeterminate-indicator-1, - :host([validation-state='success']) .indeterminate-indicator-2 { - background: linear-gradient( + :host(:not([value])) .indicator { + position: relative; + width: 33%; + background-image: linear-gradient( to right, - ${colorPaletteGreenBackground2} 0%, - ${colorPaletteGreenBackground3} 50%, - ${colorPaletteGreenBackground2} + ${colorNeutralBackground6} 0%, + ${colorTransparentBackground} 50%, + ${colorNeutralBackground6} 100% ); + animation-name: indeterminate; + animation-duration: 3s; + animation-timing-function: linear; + animation-iteration-count: infinite; } - .progress { - background-color: ${colorNeutralBackground6}; - border-radius: ${borderRadiusMedium}; - width: 100%; - height: 2px; - display: flex; - align-items: center; - position: relative; - } - - .determinate { - background-color: ${colorCompoundBrandBackground}; - border-radius: ${borderRadiusMedium}; - height: 2px; - transition: all 0.2s ease-in-out; - display: flex; + :host([validation-state='error']) .indicator { + background-color: ${colorPaletteRedBackground3}; } - .indeterminate-indicator-1 { - position: absolute; - opacity: 0; - height: 100%; - background: linear-gradient( - to right, - ${colorBrandBackground2} 0%, - ${colorCompoundBrandBackground} 50%, - ${colorBrandBackground2} - ); - border-radius: ${borderRadiusMedium}; - animation-timing-function: cubic-bezier(0.4, 0, 0.6, 1); - width: 40%; - animation: indeterminate-1 3s infinite; + :host([validation-state='warning']) .indicator { + background-color: ${colorPaletteDarkOrangeBackground3}; } - .indeterminate-indicator-2 { - position: absolute; - opacity: 0; - height: 100%; - background: linear-gradient( - to right, - ${colorBrandBackground2} 0%, - ${colorCompoundBrandBackground} 50%, - ${colorBrandBackground2} - ); - border-radius: ${borderRadiusMedium}; - animation-timing-function: cubic-bezier(0.4, 0, 0.6, 1); - width: 60%; - animation: indeterminate-2 3s infinite; + :host([validation-state='success']) .indicator { + background-color: ${colorPaletteGreenBackground3}; } - @keyframes indeterminate-1 { - 0% { - opacity: 1; - transform: translateX(-100%); - } - 70% { - opacity: 1; - transform: translateX(300%); - } - 70.01% { - opacity: 0; - } - 100% { - opacity: 0; - transform: translateX(300%); + @layer animations { + /* Disable animations for reduced motion */ + @media (prefers-reduced-motion: no-preference) { + :host([value]) { + transition: none; + } + :host(:not([value])) .indicator { + animation-duration: 0.01ms; + animation-iteration-count: 1; + } } } - @keyframes indeterminate-2 { + + @keyframes indeterminate { 0% { - opacity: 0; - transform: translateX(-150%); - } - 29.99% { - opacity: 0; - } - 30% { - opacity: 1; - transform: translateX(-150%); + inset-inline-start: -33%; } 100% { - transform: translateX(166.66%); - opacity: 1; + inset-inline-start: 100%; } } `.withBehaviors( forcedColorsStylesheetBehavior(css` - .progress { - background-color: HighlightText; + :host { + background-color: CanvasText; } - .determinate, - :host([validation-state='success']) .determinate, - :host([validation-state='warning']) .determinate, - :host([validation-state='error']) .determinate, - :host([validation-state='success']) ..indeterminate-indicator-1, - :host([validation-state='success']) ..indeterminate-indicator-2, - :host([validation-state='warning']) .indeterminate-indicator-1, - :host([validation-state='warning']) .indeterminate-indicator-2, - :host([validation-state='error']) .indeterminate-indicator-1, - :host([validation-state='error']) .indeterminate-indicator-2 { + .indicator, + :host(:is([validation-state='success'], [validation-state='warning'], [validation-state='error'])) .indicator { background-color: Highlight; } `), diff --git a/packages/web-components/src/progress-bar/progress-bar.template.ts b/packages/web-components/src/progress-bar/progress-bar.template.ts index 9be9714c5c959..f81062f5f8c5c 100644 --- a/packages/web-components/src/progress-bar/progress-bar.template.ts +++ b/packages/web-components/src/progress-bar/progress-bar.template.ts @@ -1,38 +1,15 @@ -import { html, when } from '@microsoft/fast-element'; +import { html } from '@microsoft/fast-element'; import type { ElementViewTemplate } from '@microsoft/fast-element'; -import { staticallyCompose } from '../utils/template-helpers.js'; -import type { ProgressOptions } from './progress-bar.options.js'; import type { ProgressBar } from './progress-bar.js'; -export function progressTemplate(options: ProgressOptions = {}): ElementViewTemplate { +export function progressTemplate(): ElementViewTemplate { return html` - +
`; } -export const template: ElementViewTemplate = progressTemplate({ - indeterminateIndicator1: ``, -}); +export const template: ElementViewTemplate = progressTemplate(); diff --git a/packages/web-components/src/progress-bar/progress-bar.ts b/packages/web-components/src/progress-bar/progress-bar.ts index c582f6d1b1625..e5b5c725bd200 100644 --- a/packages/web-components/src/progress-bar/progress-bar.ts +++ b/packages/web-components/src/progress-bar/progress-bar.ts @@ -1,5 +1,4 @@ -import { attr } from '@microsoft/fast-element'; -import { BaseProgress } from './base-progress.js'; +import { attr, FASTElement, nullableNumberConverter, observable } from '@microsoft/fast-element'; import { ProgressBarShape, ProgressBarThickness, ProgressBarValidationState } from './progress-bar.options.js'; /** @@ -13,12 +12,18 @@ import { ProgressBarShape, ProgressBarThickness, ProgressBarValidationState } fr * * @public */ -export class ProgressBar extends BaseProgress { +export class ProgressBar extends FASTElement { + /** + * The internal {@link https://developer.mozilla.org/docs/Web/API/ElementInternals | `ElementInternals`} instance for the component. + * + * @internal + */ + public elementInternals: ElementInternals = this.attachInternals(); + /** * The thickness of the progress bar * * @public - * @remarks * HTML Attribute: thickness */ @attr @@ -27,7 +32,6 @@ export class ProgressBar extends BaseProgress { /** * The shape of the progress bar * @public - * @remarks * HTML Attribute: shape */ @attr @@ -36,9 +40,96 @@ export class ProgressBar extends BaseProgress { /** * The validation state of the progress bar * @public - * @remarks * HTML Attribute: validation-state */ @attr({ attribute: 'validation-state' }) public validationState: ProgressBarValidationState | null = null; + + /** + * The value of the progress + * @internal + * HTML Attribute: value + */ + @attr({ converter: nullableNumberConverter }) + public value?: number | null; + protected valueChanged(): void { + this.updateAriaValueNow(); + this.updatePercentComplete(); + } + + /** + * The minimum value + * @internal + * HTML Attribute: min + */ + @attr({ converter: nullableNumberConverter }) + public min?: number; + protected minChanged(): void { + if (this.$fastController.isConnected) { + this.updateAriaValueMin(); + this.updatePercentComplete(); + } + } + + /** + * The maximum value + * @internal + * HTML Attribute: max + */ + @attr({ converter: nullableNumberConverter }) + public max?: number; + protected maxChanged(): void { + if (this.$fastController.isConnected) { + this.updateAriaValueMax(); + this.updatePercentComplete(); + } + } + + /** + * Indicates progress in % + * @internal + */ + @observable + public percentComplete: number = 0; + + /** + * @internal + */ + public constructor() { + super(); + + this.elementInternals.role = 'progressbar'; + } + + /** + * @internal + */ + public connectedCallback(): void { + super.connectedCallback(); + this.updateAriaValueNow(); + this.updateAriaValueMin(); + this.updateAriaValueMax(); + this.updatePercentComplete(); + } + + private updateAriaValueMax(): void { + this.elementInternals.ariaValueMax = this.max ? `${this.max}` : null; + } + + private updateAriaValueMin(): void { + this.elementInternals.ariaValueMin = this.min ? `${this.min}` : null; + } + + private updateAriaValueNow(): void { + this.elementInternals.ariaValueNow = this.value ? `${this.value}` : null; + } + + private updatePercentComplete(): void { + const min: number = typeof this.min === 'number' ? this.min : 0; + const max: number = typeof this.max === 'number' ? this.max : 100; + const value: number = typeof this.value === 'number' ? this.value : 0; + const range: number = max - min; + + this.percentComplete = range === 0 ? 0 : Math.fround(((value - min) / range) * 100); + } } From a4762b44ec4416d517099cd8915da0c2d695ba26 Mon Sep 17 00:00:00 2001 From: Chris Holt <13071055+chrisdholt@users.noreply.github.com> Date: Mon, 10 Jun 2024 19:02:34 -0700 Subject: [PATCH 2/7] remove calls to set aria values in connected callback and remove tsdoc from constructor and connectedCallback --- .../web-components/src/progress-bar/progress-bar.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/web-components/src/progress-bar/progress-bar.ts b/packages/web-components/src/progress-bar/progress-bar.ts index e5b5c725bd200..6229e0564e6b0 100644 --- a/packages/web-components/src/progress-bar/progress-bar.ts +++ b/packages/web-components/src/progress-bar/progress-bar.ts @@ -92,23 +92,15 @@ export class ProgressBar extends FASTElement { @observable public percentComplete: number = 0; - /** - * @internal - */ public constructor() { super(); this.elementInternals.role = 'progressbar'; } - /** - * @internal - */ public connectedCallback(): void { super.connectedCallback(); - this.updateAriaValueNow(); - this.updateAriaValueMin(); - this.updateAriaValueMax(); + this.updatePercentComplete(); } From 538747bd628a78babeda4dc37621bb11dada19df Mon Sep 17 00:00:00 2001 From: Chris Holt <13071055+chrisdholt@users.noreply.github.com> Date: Mon, 10 Jun 2024 19:05:58 -0700 Subject: [PATCH 3/7] use nullish coalescing operator --- packages/web-components/docs/api-report.md | 3 +-- .../src/progress-bar/progress-bar.spec.ts | 4 ++-- .../src/progress-bar/progress-bar.ts | 23 ++++++++----------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/packages/web-components/docs/api-report.md b/packages/web-components/docs/api-report.md index d366c71a38266..4b62822c797da 100644 --- a/packages/web-components/docs/api-report.md +++ b/packages/web-components/docs/api-report.md @@ -2438,9 +2438,8 @@ export const MenuTemplate: ElementViewTemplate; // @public export class ProgressBar extends FASTElement { - // @internal constructor(); - // @internal (undocumented) + // (undocumented) connectedCallback(): void; // @internal elementInternals: ElementInternals; diff --git a/packages/web-components/src/progress-bar/progress-bar.spec.ts b/packages/web-components/src/progress-bar/progress-bar.spec.ts index 5db519b56836e..5b6bffb472056 100644 --- a/packages/web-components/src/progress-bar/progress-bar.spec.ts +++ b/packages/web-components/src/progress-bar/progress-bar.spec.ts @@ -44,7 +44,7 @@ test.describe('Progress Bar', () => { `; }); - await expect(element).toHaveJSProperty('ariaValueMin', '50'); + await expect(element).toHaveJSProperty('elementInternals.ariaValueMin', '50'); }); test('should set the `aria-valuemax` attribute with the `max` property when provided', async () => { @@ -54,7 +54,7 @@ test.describe('Progress Bar', () => { `; }); - await expect(element).toHaveJSProperty('ariaValueMax', '50'); + await expect(element).toHaveJSProperty('elementInternals.ariaValueMax', '50'); }); test('should return the `percentComplete` property as a value between 0 and 100 when `min` and `max` are unset', async () => { diff --git a/packages/web-components/src/progress-bar/progress-bar.ts b/packages/web-components/src/progress-bar/progress-bar.ts index 6229e0564e6b0..63749e6e23dae 100644 --- a/packages/web-components/src/progress-bar/progress-bar.ts +++ b/packages/web-components/src/progress-bar/progress-bar.ts @@ -5,11 +5,6 @@ import { ProgressBarShape, ProgressBarThickness, ProgressBarValidationState } fr * An Progress HTML Element. * Implements the {@link https://www.w3.org/TR/wai-aria-1.1/#progressbar | ARIA progressbar }. * - * @slot indeterminate - The slot for a custom indeterminate indicator - * @csspart progress - Represents the progress element - * @csspart determinate - The determinate indicator - * @csspart indeterminate - The indeterminate indicator - * * @public */ export class ProgressBar extends FASTElement { @@ -24,7 +19,7 @@ export class ProgressBar extends FASTElement { * The thickness of the progress bar * * @public - * HTML Attribute: thickness + * HTML Attribute: `thickness` */ @attr public thickness?: ProgressBarThickness; @@ -32,7 +27,7 @@ export class ProgressBar extends FASTElement { /** * The shape of the progress bar * @public - * HTML Attribute: shape + * HTML Attribute: `shape` */ @attr public shape?: ProgressBarShape; @@ -40,7 +35,7 @@ export class ProgressBar extends FASTElement { /** * The validation state of the progress bar * @public - * HTML Attribute: validation-state + * HTML Attribute: `validation-state` */ @attr({ attribute: 'validation-state' }) public validationState: ProgressBarValidationState | null = null; @@ -48,7 +43,7 @@ export class ProgressBar extends FASTElement { /** * The value of the progress * @internal - * HTML Attribute: value + * HTML Attribute: `value` */ @attr({ converter: nullableNumberConverter }) public value?: number | null; @@ -60,7 +55,7 @@ export class ProgressBar extends FASTElement { /** * The minimum value * @internal - * HTML Attribute: min + * HTML Attribute: `min` */ @attr({ converter: nullableNumberConverter }) public min?: number; @@ -74,7 +69,7 @@ export class ProgressBar extends FASTElement { /** * The maximum value * @internal - * HTML Attribute: max + * HTML Attribute: `max` */ @attr({ converter: nullableNumberConverter }) public max?: number; @@ -117,9 +112,9 @@ export class ProgressBar extends FASTElement { } private updatePercentComplete(): void { - const min: number = typeof this.min === 'number' ? this.min : 0; - const max: number = typeof this.max === 'number' ? this.max : 100; - const value: number = typeof this.value === 'number' ? this.value : 0; + const min: number = this.min ?? 0; + const max: number = this.max ?? 100; + const value: number = this.value ?? 0; const range: number = max - min; this.percentComplete = range === 0 ? 0 : Math.fround(((value - min) / range) * 100); From 82cc22a3fa1d94cb903d00d0dc31ed01524f4ef2 Mon Sep 17 00:00:00 2001 From: Chris Holt <13071055+chrisdholt@users.noreply.github.com> Date: Tue, 11 Jun 2024 09:05:39 -0700 Subject: [PATCH 4/7] add aria udpates back to connectedCallback --- packages/web-components/src/progress-bar/progress-bar.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/web-components/src/progress-bar/progress-bar.ts b/packages/web-components/src/progress-bar/progress-bar.ts index 63749e6e23dae..0cf25dc781183 100644 --- a/packages/web-components/src/progress-bar/progress-bar.ts +++ b/packages/web-components/src/progress-bar/progress-bar.ts @@ -96,6 +96,9 @@ export class ProgressBar extends FASTElement { public connectedCallback(): void { super.connectedCallback(); + this.updateAriaValueMin(); + this.updateAriaValueMax(); + this.updateAriaValueNow(); this.updatePercentComplete(); } From 8efbd4212fe3f486ad7b3bdf60ea85d013503eaa Mon Sep 17 00:00:00 2001 From: John Kreitlow <863023+radium-v@users.noreply.github.com> Date: Tue, 11 Jun 2024 10:56:53 -0700 Subject: [PATCH 5/7] make progress-bar tests async --- .../src/progress-bar/progress-bar.spec.ts | 99 +++++++++---------- 1 file changed, 48 insertions(+), 51 deletions(-) diff --git a/packages/web-components/src/progress-bar/progress-bar.spec.ts b/packages/web-components/src/progress-bar/progress-bar.spec.ts index 5b6bffb472056..65503d977f6d9 100644 --- a/packages/web-components/src/progress-bar/progress-bar.spec.ts +++ b/packages/web-components/src/progress-bar/progress-bar.spec.ts @@ -1,78 +1,70 @@ import { expect, test } from '@playwright/test'; -import type { Locator, Page } from '@playwright/test'; import { fixtureURL } from '../helpers.tests.js'; import type { ProgressBar } from './progress-bar.js'; test.describe('Progress Bar', () => { - let page: Page; - let element: Locator; - let root: Locator; - - test.beforeAll(async ({ browser }) => { - page = await browser.newPage(); - - element = page.locator('fluent-progress-bar'); - - root = page.locator('#root'); - + test.beforeEach(async ({ page }) => { await page.goto(fixtureURL('components-progressbar--progress')); - }); - test.afterAll(async () => { - await page.close(); + await page.waitForFunction(() => customElements.whenDefined('fluent-progress-bar')); }); - // Foundation tests - test('should include a role of progressbar', async () => { + test('should include a role of progressbar', async ({ page }) => { + const element = page.locator('fluent-progress-bar'); + await expect(element).toHaveJSProperty('elementInternals.role', 'progressbar'); }); - test('should set the `aria-valuenow` attribute with the `value` property when provided', async () => { - await root.evaluate(node => { - node.innerHTML = /* html */ ` - - `; - }); + test('should set the `aria-valuenow` attribute with the `value` property when provided', async ({ page }) => { + const element = page.locator('fluent-progress-bar'); + + await page.setContent(/* html */ ` + + `); await expect(element).toHaveJSProperty('elementInternals.ariaValueNow', '50'); }); - test('should set the `aria-valuemin` attribute with the `min` property when provided', async () => { - await root.evaluate(node => { - node.innerHTML = /* html */ ` - - `; - }); + test('should set the `aria-valuemin` attribute with the `min` property when provided', async ({ page }) => { + const element = page.locator('fluent-progress-bar'); + + await page.setContent(/* html */ ` + + `); await expect(element).toHaveJSProperty('elementInternals.ariaValueMin', '50'); }); - test('should set the `aria-valuemax` attribute with the `max` property when provided', async () => { - await root.evaluate(node => { - node.innerHTML = /* html */ ` - - `; - }); + test('should set the `aria-valuemax` attribute with the `max` property when provided', async ({ page }) => { + const element = page.locator('fluent-progress-bar'); + + await page.setContent(/* html */ ` + + `); await expect(element).toHaveJSProperty('elementInternals.ariaValueMax', '50'); }); - test('should return the `percentComplete` property as a value between 0 and 100 when `min` and `max` are unset', async () => { - await root.evaluate(node => { - node.innerHTML = /* html */ ` - - `; - }); + test('should return the `percentComplete` property as a value between 0 and 100 when `min` and `max` are unset', async ({ + page, + }) => { + const element = page.locator('fluent-progress-bar'); + + await page.setContent(/* html */ ` + + `); await expect(element).toHaveJSProperty('percentComplete', 50); }); - test('should set the `percentComplete` property to match the current `value` in the range of `min` and `max`', async () => { - await root.evaluate(node => { - node.innerHTML = /* html */ ` - - `; - }); + test('should set the `percentComplete` property to match the current `value` in the range of `min` and `max`', async ({ + page, + }) => { + const element = page.locator('fluent-progress-bar'); + + await page.setContent(/* html */ ` + + `); await expect(element).toHaveJSProperty('percentComplete', 0); @@ -101,8 +93,9 @@ test.describe('Progress Bar', () => { await expect(element).toHaveJSProperty('percentComplete', 0); }); - // Fluent Specific propertiy tests - test('should set and retrieve the `thickness` property correctly', async () => { + test('should set and retrieve the `thickness` property correctly', async ({ page }) => { + const element = page.locator('fluent-progress-bar'); + await element.evaluate((node: ProgressBar) => { node.thickness = 'medium'; }); @@ -116,7 +109,9 @@ test.describe('Progress Bar', () => { await expect(element).toHaveJSProperty('thickness', 'large'); }); - test('should set and retrieve the `shape` property correctly', async () => { + test('should set and retrieve the `shape` property correctly', async ({ page }) => { + const element = page.locator('fluent-progress-bar'); + await element.evaluate((node: ProgressBar) => { node.shape = 'square'; }); @@ -130,7 +125,9 @@ test.describe('Progress Bar', () => { await expect(element).toHaveJSProperty('shape', 'rounded'); }); - test('should set and retrieve the `validationState` property correctly', async () => { + test('should set and retrieve the `validationState` property correctly', async ({ page }) => { + const element = page.locator('fluent-progress-bar'); + await element.evaluate((node: ProgressBar) => { node.validationState = 'success'; }); From 2611e1484d70498c361ba9630b61278a04a668da Mon Sep 17 00:00:00 2001 From: John Kreitlow <863023+radium-v@users.noreply.github.com> Date: Tue, 11 Jun 2024 11:31:08 -0700 Subject: [PATCH 6/7] Simplify property change methods and use volatile for percentComplete --- .../src/progress-bar/progress-bar.ts | 85 +++++++++---------- 1 file changed, 38 insertions(+), 47 deletions(-) diff --git a/packages/web-components/src/progress-bar/progress-bar.ts b/packages/web-components/src/progress-bar/progress-bar.ts index 0cf25dc781183..c432005cd5755 100644 --- a/packages/web-components/src/progress-bar/progress-bar.ts +++ b/packages/web-components/src/progress-bar/progress-bar.ts @@ -1,4 +1,4 @@ -import { attr, FASTElement, nullableNumberConverter, observable } from '@microsoft/fast-element'; +import { attr, FASTElement, nullableNumberConverter, volatile } from '@microsoft/fast-element'; import { ProgressBarShape, ProgressBarThickness, ProgressBarValidationState } from './progress-bar.options.js'; /** @@ -46,10 +46,15 @@ export class ProgressBar extends FASTElement { * HTML Attribute: `value` */ @attr({ converter: nullableNumberConverter }) - public value?: number | null; - protected valueChanged(): void { - this.updateAriaValueNow(); - this.updatePercentComplete(); + public value?: number; + + /** + * Updates the percent complete when the `value` property changes. + * + * @internal + */ + protected valueChanged(prev: number | undefined, next: number | undefined): void { + this.elementInternals.ariaValueNow = typeof next === 'number' ? `${next}` : null; } /** @@ -59,11 +64,15 @@ export class ProgressBar extends FASTElement { */ @attr({ converter: nullableNumberConverter }) public min?: number; - protected minChanged(): void { - if (this.$fastController.isConnected) { - this.updateAriaValueMin(); - this.updatePercentComplete(); - } + + /** + * Updates the percent complete when the `min` property changes. + * + * @param prev - The previous min value + * @param next - The current min value + */ + protected minChanged(prev: number | undefined, next: number | undefined): void { + this.elementInternals.ariaValueMin = typeof next === 'number' ? `${next}` : null; } /** @@ -73,53 +82,35 @@ export class ProgressBar extends FASTElement { */ @attr({ converter: nullableNumberConverter }) public max?: number; - protected maxChanged(): void { - if (this.$fastController.isConnected) { - this.updateAriaValueMax(); - this.updatePercentComplete(); - } + + /** + * Updates the percent complete when the `max` property changes. + * + * @param prev - The previous max value + * @param next - The current max value + * @internal + */ + protected maxChanged(prev: number | undefined, next: number | undefined): void { + this.elementInternals.ariaValueMax = typeof next === 'number' ? `${next}` : null; } /** * Indicates progress in % * @internal */ - @observable - public percentComplete: number = 0; + @volatile + public get percentComplete(): number { + const min = this.min ?? 0; + const max = this.max ?? 100; + const value = this.value ?? 0; + const range = max - min; + + return range === 0 ? 0 : Math.fround(((value - min) / range) * 100); + } public constructor() { super(); this.elementInternals.role = 'progressbar'; } - - public connectedCallback(): void { - super.connectedCallback(); - - this.updateAriaValueMin(); - this.updateAriaValueMax(); - this.updateAriaValueNow(); - this.updatePercentComplete(); - } - - private updateAriaValueMax(): void { - this.elementInternals.ariaValueMax = this.max ? `${this.max}` : null; - } - - private updateAriaValueMin(): void { - this.elementInternals.ariaValueMin = this.min ? `${this.min}` : null; - } - - private updateAriaValueNow(): void { - this.elementInternals.ariaValueNow = this.value ? `${this.value}` : null; - } - - private updatePercentComplete(): void { - const min: number = this.min ?? 0; - const max: number = this.max ?? 100; - const value: number = this.value ?? 0; - const range: number = max - min; - - this.percentComplete = range === 0 ? 0 : Math.fround(((value - min) / range) * 100); - } } From f574a436e075c8ec891e83fb30b6f75492740d6e Mon Sep 17 00:00:00 2001 From: John Kreitlow <863023+radium-v@users.noreply.github.com> Date: Tue, 11 Jun 2024 11:31:23 -0700 Subject: [PATCH 7/7] fix typo --- .../web-components/src/progress-bar/progress-bar.stories.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web-components/src/progress-bar/progress-bar.stories.ts b/packages/web-components/src/progress-bar/progress-bar.stories.ts index 762e7daa4e6ff..9e697bf08485f 100644 --- a/packages/web-components/src/progress-bar/progress-bar.stories.ts +++ b/packages/web-components/src/progress-bar/progress-bar.stories.ts @@ -74,7 +74,7 @@ export const Max = renderComponent(html`

- 3 o 5 + 3 of 5