Remove banner dismissButtonLabel (#1376)

# Pull Request ## 🤨 Rationale Follow-up tech debt from adding the label providers (#1090). The banner's close button text can now be controlled by the `popupDismiss` label on `nimble-label-provider-core`, so the `dismissButtonLabel` property on `nimble-banner` is no longer needed. We decided to remove the property in [PR discussion on #1328](#1328 (comment)). ## 👩‍💻 Implementation Remove property and update tests. ## 🧪 Testing Updated autotests / Storybook. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed.
ni · Jul 25, 2023 · 2c77e8e · 2c77e8e
1 parent f2c5b90
commit 2c77e8e
Show file tree

Hide file tree

Showing 12 changed files with 27 additions and 88 deletions.
diff --git a/...lar-workspace/projects/ni/nimble-angular/src/directives/banner/nimble-banner.directive.ts b/...lar-workspace/projects/ni/nimble-angular/src/directives/banner/nimble-banner.directive.ts
@@ -53,16 +53,6 @@ export class NimbleBannerDirective {
         this.renderer.setProperty(this.elementRef.nativeElement, 'preventDismiss', toBooleanProperty(value));
     }
 
-    public get dismissButtonLabel(): string | undefined {
-        return this.elementRef.nativeElement.dismissButtonLabel;
-    }
-
-    // Renaming because property should have camel casing, but attribute should not
-    // eslint-disable-next-line @angular-eslint/no-input-rename
-    @Input('dismiss-button-label') public set dismissButtonLabel(value: string | undefined) {
-        this.renderer.setProperty(this.elementRef.nativeElement, 'dismissButtonLabel', value);
-    }
-
     public constructor(private readonly renderer: Renderer2, private readonly elementRef: ElementRef<Banner>) {}
 
     @HostListener('toggle', ['$event'])

diff --git a/...ce/projects/ni/nimble-angular/src/directives/banner/tests/nimble-banner.directive.spec.ts b/...ce/projects/ni/nimble-angular/src/directives/banner/tests/nimble-banner.directive.spec.ts
@@ -61,11 +61,6 @@ describe('Nimble banner', () => {
             expect(directive.preventDismiss).toBeFalse();
             expect(nativeElement.preventDismiss).toBeFalse();
         });
-
-        it('has expected defaults for dismissButtonLabel', () => {
-            expect(directive.dismissButtonLabel).toBeUndefined();
-            expect(nativeElement.dismissButtonLabel).toBeUndefined();
-        });
     });
 
     describe('with template string values', () => {
@@ -75,8 +70,7 @@ describe('Nimble banner', () => {
                     open
                     severity="error"
                     title-hidden
-                    prevent-dismiss
-                    dismiss-button-label="Dismiss">
+                    prevent-dismiss>
                 </nimble-banner>`
         })
         class TestHostComponent {
@@ -118,11 +112,6 @@ describe('Nimble banner', () => {
             expect(directive.preventDismiss).toBeTrue();
             expect(nativeElement.preventDismiss).toBeTrue();
         });
-
-        it('will use template string values for dismissButtonLabel', () => {
-            expect(directive.dismissButtonLabel).toBe('Dismiss');
-            expect(nativeElement.dismissButtonLabel).toBe('Dismiss');
-        });
     });
 
     describe('with property bound values', () => {
@@ -132,8 +121,7 @@ describe('Nimble banner', () => {
                     [open]="open"
                     [severity]="severity"
                     [title-hidden]="titleHidden"
-                    [prevent-dismiss]="preventDismiss"
-                    [dismiss-button-label]="dismissButtonLabel">
+                    [prevent-dismiss]="preventDismiss">
                 </nimble-banner>`
         })
         class TestHostComponent {
@@ -143,7 +131,6 @@ describe('Nimble banner', () => {
             public severity: BannerSeverity = BannerSeverity.warning;
             public titleHidden = false;
             public preventDismiss = false;
-            public dismissButtonLabel = 'Dismiss';
         }
 
         let fixture: ComponentFixture<TestHostComponent>;
@@ -204,17 +191,6 @@ describe('Nimble banner', () => {
             expect(directive.preventDismiss).toBeTrue();
             expect(nativeElement.preventDismiss).toBeTrue();
         });
-
-        it('can be configured with property binding for dismissButtonLabel', () => {
-            expect(directive.dismissButtonLabel).toBe('Dismiss');
-            expect(nativeElement.dismissButtonLabel).toBe('Dismiss');
-
-            fixture.componentInstance.dismissButtonLabel = 'Close';
-            fixture.detectChanges();
-
-            expect(directive.dismissButtonLabel).toBe('Close');
-            expect(nativeElement.dismissButtonLabel).toBe('Close');
-        });
     });
 
     describe('with attribute bound values', () => {
@@ -224,8 +200,7 @@ describe('Nimble banner', () => {
                     [attr.open]="open"
                     [attr.severity]="severity"
                     [attr.title-hidden]="titleHidden"
-                    [attr.prevent-dismiss]="preventDismiss"
-                    [attr.dismiss-button-label]="dismissButtonLabel">
+                    [attr.prevent-dismiss]="preventDismiss">
                 </nimble-banner>`
         })
         class TestHostComponent {
@@ -235,7 +210,6 @@ describe('Nimble banner', () => {
             public severity: BannerSeverity = BannerSeverity.warning;
             public titleHidden = false;
             public preventDismiss = false;
-            public dismissButtonLabel = 'Dismiss';
         }
 
         let fixture: ComponentFixture<TestHostComponent>;
@@ -296,16 +270,5 @@ describe('Nimble banner', () => {
             expect(directive.preventDismiss).toBeTrue();
             expect(nativeElement.preventDismiss).toBeTrue();
         });
-
-        it('can be configured with attribute binding for dismissButtonLabel', () => {
-            expect(directive.dismissButtonLabel).toBe('Dismiss');
-            expect(nativeElement.dismissButtonLabel).toBe('Dismiss');
-
-            fixture.componentInstance.dismissButtonLabel = 'Close';
-            fixture.detectChanges();
-
-            expect(directive.dismissButtonLabel).toBe('Close');
-            expect(nativeElement.dismissButtonLabel).toBe('Close');
-        });
     });
 });
diff --git a/change/@ni-nimble-angular-4195d860-6f9d-49d1-83bc-7ccc1b77743d.json b/change/@ni-nimble-angular-4195d860-6f9d-49d1-83bc-7ccc1b77743d.json
@@ -0,0 +1,7 @@
+{
+  "type": "major",
+  "comment": "Remove nimble-banner dismissButtonLabel (replaced by the popupDismiss label on nimble-label-provider-core)",
+  "packageName": "@ni/nimble-angular",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@ni-nimble-blazor-6afc7e92-3600-498c-b5a4-e6a8088d0c58.json b/change/@ni-nimble-blazor-6afc7e92-3600-498c-b5a4-e6a8088d0c58.json
@@ -0,0 +1,7 @@
+{
+  "type": "major",
+  "comment": "Remove nimble-banner dismissButtonLabel (replaced by the popupDismiss label on nimble-label-provider-core)",
+  "packageName": "@ni/nimble-blazor",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@ni-nimble-components-cf02f6e7-0390-47bb-b5e0-d1dd32c406b7.json b/change/@ni-nimble-components-cf02f6e7-0390-47bb-b5e0-d1dd32c406b7.json
@@ -0,0 +1,7 @@
+{
+  "type": "major",
+  "comment": "Remove nimble-banner dismissButtonLabel (replaced by the popupDismiss label on nimble-label-provider-core)",
+  "packageName": "@ni/nimble-components",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleBanner.razor b/packages/nimble-blazor/NimbleBlazor/Components/NimbleBanner.razor
@@ -5,7 +5,6 @@
     severity="@Severity.ToAttributeValue()"
     title-hidden="@TitleHidden"
     prevent-dismiss="@PreventDismiss"
-    dismiss-button-label="@DismissButtonLabel"
     @attributes="AdditionalAttributes">
     @ChildContent
 </nimble-banner>
diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleBanner.razor.cs b/packages/nimble-blazor/NimbleBlazor/Components/NimbleBanner.razor.cs
@@ -31,12 +31,6 @@ public partial class NimbleBanner : ComponentBase
     [Parameter]
     public bool? PreventDismiss { get; set; }
 
-    /// <summary>
-    /// The (hidden) label applied to the dismiss button (for accessibility purposes)
-    /// </summary>
-    [Parameter]
-    public string? DismissButtonLabel { get; set; }
-
     /// <summary>
     /// The child content of the element.
     /// </summary>

diff --git a/packages/nimble-components/src/banner/index.ts b/packages/nimble-components/src/banner/index.ts
@@ -51,14 +51,6 @@ export class Banner extends FoundationElement {
     @attr({ attribute: 'prevent-dismiss', mode: 'boolean' })
     public preventDismiss = false;
 
-    /**
-     * @public
-     * @description
-     * Label (not visible) for the dismiss button
-     */
-    @attr({ attribute: 'dismiss-button-label' })
-    public dismissButtonLabel?: string;
-
     /**
      * @internal
      */

diff --git a/packages/nimble-components/src/banner/specs/README.md b/packages/nimble-components/src/banner/specs/README.md
@@ -50,7 +50,6 @@ _The key elements of the component's public API surface:_
     -   `prevent-dismiss` - set to hide the dismiss button (attr name taken from Nimble dialog)
     -   `severity` - one of `error`, `warning`, `info`, or undefined (default)
     -   `title-hidden` - set to hide the title text, which should always be provided for a11y reasons
-    -   `dismiss-button-label` - a localized label to give the dismiss button (for a11y purposes)
 -   _Methods_
 -   _Events_
     -   `toggle` - fired when the banner is closed or opened. Event has `newState` and `oldState`.
@@ -167,7 +166,7 @@ _Consider the accessibility of the component, including:_
 
 ### Globalization
 
-N/A
+Localization: The banner will use the `popupDismiss` label on `nimble-label-provider-core` for the dismiss button label text (hidden visually, but present for accessibility).
 
 ### Security
 

diff --git a/packages/nimble-components/src/banner/template.ts b/packages/nimble-components/src/banner/template.ts
@@ -54,7 +54,7 @@ export const template = html<Banner>`
                 ${when(x => !x.preventDismiss, html<Banner>`
                     <${buttonTag} appearance="ghost" content-hidden @click="${x => x.dismissBanner()}">
                         <${iconXmarkTag} slot="start"></${iconXmarkTag}>
-                        ${x => x.dismissButtonLabel ?? popupDismissLabel.getValueFor(x)}
+                        ${x => popupDismissLabel.getValueFor(x)}
                     </${buttonTag}>
                 `)}
             </div>

diff --git a/packages/nimble-components/src/banner/tests/banner.spec.ts b/packages/nimble-components/src/banner/tests/banner.spec.ts
@@ -102,16 +102,6 @@ describe('Banner', () => {
         ).toBeTrue();
     });
 
-    it("should set 'dismissButtonLabel' as label of dismiss button", async () => {
-        element.dismissButtonLabel = 'Dismiss';
-        await waitForUpdatesAsync();
-        expect(
-            element.shadowRoot
-                ?.querySelector('nimble-button')
-                ?.innerText.includes('Dismiss')
-        ).toBeTrue();
-    });
-
     it("should set the 'role' to 'status'", () => {
         expect(
             element.shadowRoot
@@ -143,7 +133,7 @@ describe('Banner with LabelProviderCore', () => {
         await disconnect();
     });
 
-    it('uses CoreLabelProvider popupDismissLabel for the close button label when dismissButtonLabel is unset', async () => {
+    it('uses CoreLabelProvider popupDismissLabel for the close button label', async () => {
         labelProvider.popupDismiss = 'Customized Close';
         await waitForUpdatesAsync();
 

diff --git a/packages/nimble-components/src/banner/tests/banner.stories.ts b/packages/nimble-components/src/banner/tests/banner.stories.ts
@@ -34,7 +34,6 @@ interface BannerArgs extends LabelUserArgs {
     action: ActionType;
     preventDismiss: boolean;
     titleHidden: boolean;
-    dismissButtonLabel: string;
     toggle: unknown;
 }
 
@@ -73,7 +72,6 @@ export const _banner: StoryObj<BannerArgs> = {
             severity="${x => BannerSeverity[x.severity]}"
             ?title-hidden="${x => x.titleHidden}"
             ?prevent-dismiss="${x => x.preventDismiss}"
-            dismiss-button-label="Close"
         >
             <span slot="title">${x => x.title}</span>
             ${x => x.text}
@@ -125,12 +123,6 @@ export const _banner: StoryObj<BannerArgs> = {
             name: 'title-hidden',
             description: 'If set, hides the provided title.'
         },
-        dismissButtonLabel: {
-            name: 'dismiss-button-label',
-            description:
-                'Set to a localized label (e.g. `"Close"`) for the dismiss button. This provides an accessible name for assistive technologies. <br>(Equivalent to setting `popup-dismiss` on `nimble-label-provider-core`)',
-            control: { type: 'none' }
-        },
         toggle: {
             description:
                 'Event emitted by the banner when the `open` state changes. The event details include the booleans `oldState` and `newState`.',
@@ -145,7 +137,6 @@ export const _banner: StoryObj<BannerArgs> = {
         action: 'none',
         preventDismiss: false,
         titleHidden: false,
-        dismissButtonLabel: undefined,
         toggle: undefined
     }
 };