refactor: allow setting popover role directly (#9869)

Author: sissbruecker

packages/popover/src/vaadin-popover.js

@@ -314,14 +314,22 @@ class Popover extends PopoverPositionMixin(
         observer: '__openedChanged',
       },
 
+      /**
+       * The `role` attribute value to be set on the popover.
+       * When not specified, defaults to 'dialog'.
+       */
+      role: {
+        type: String,
+        reflectToAttribute: true,
+      },
+
       /**
        * The `role` attribute value to be set on the overlay.
        *
        * @attr {string} overlay-role
        */
       overlayRole: {
         type: String,
-        value: 'dialog',
       },
 
       /**
@@ -413,7 +421,7 @@ class Popover extends PopoverPositionMixin(
   }
 
   static get observers() {
-    return ['__sizeChanged(width, height, _overlayElement)', '__updateAriaAttributes(opened, overlayRole, target)'];
+    return ['__sizeChanged(width, height, _overlayElement)', '__updateAriaAttributes(opened, role, target)'];
   }
 
   /**
@@ -520,15 +528,24 @@ class Popover extends PopoverPositionMixin(
     super.ready();
 
     this._overlayElement = this.$.overlay;
+
+    if (!this.hasAttribute('role')) {
+      this.role = 'dialog';
+    }
   }
 
   /** @protected */
-  updated(props) {
-    super.updated(props);
+  willUpdate(props) {
+    super.willUpdate(props);
 
     if (props.has('overlayRole')) {
-      this.setAttribute('role', this.overlayRole);
+      this.role = this.overlayRole;
     }
+  }
+
+  /** @protected */
+  updated(props) {
+    super.updated(props);
 
     if (props.has('accessibleName')) {
       if (this.accessibleName) {
@@ -610,7 +627,7 @@ class Popover extends PopoverPositionMixin(
   }
 
   /** @private */
-  __updateAriaAttributes(opened, overlayRole, target) {
+  __updateAriaAttributes(opened, role, target) {
     if (this.__oldTarget) {
       const oldEffectiveTarget = this.__oldTarget.ariaTarget || this.__oldTarget;
       oldEffectiveTarget.removeAttribute('aria-haspopup');
@@ -621,7 +638,7 @@ class Popover extends PopoverPositionMixin(
     if (target) {
       const effectiveTarget = target.ariaTarget || target;
 
-      const isDialog = overlayRole === 'dialog' || overlayRole === 'alertdialog';
+      const isDialog = role === 'dialog' || role === 'alertdialog';
       effectiveTarget.setAttribute('aria-haspopup', isDialog ? 'dialog' : 'true');
 
       effectiveTarget.setAttribute('aria-expanded', opened ? 'true' : 'false');

packages/popover/test/a11y.test.js

@@ -67,11 +67,23 @@ describe('a11y', () => {
   });
 
   describe('ARIA attributes', () => {
-    it('should set role attribute on the host element to dialog', () => {
+    it('should set role attribute to dialog', () => {
       expect(popover.getAttribute('role')).to.equal('dialog');
     });
 
-    it('should change role attribute on the host element based on overlayRole', async () => {
+    it('should allow setting role declaratively', () => {
+      popover = fixtureSync('<vaadin-popover role="menu"></vaadin-popover>');
+      expect(popover.role).to.equal('menu');
+      expect(popover.getAttribute('role')).to.equal('menu');
+    });
+
+    it('should change role attribute when setting role', async () => {
+      popover.role = 'alertdialog';
+      await nextUpdate(popover);
+      expect(popover.getAttribute('role')).to.equal('alertdialog');
+    });
+
+    it('should change role attribute based on overlayRole', async () => {
       popover.overlayRole = 'alertdialog';
       await nextUpdate(popover);
       expect(popover.getAttribute('role')).to.equal('alertdialog');
@@ -97,12 +109,24 @@ describe('a11y', () => {
           expect(element.getAttribute('aria-haspopup')).to.equal('dialog');
         });
 
+        it(`should keep aria-haspopup attribute on the ${prop} when role is set to alertdialog`, async () => {
+          popover.role = 'alertdialog';
+          await nextUpdate(popover);
+          expect(element.getAttribute('aria-haspopup')).to.equal('dialog');
+        });
+
         it(`should keep aria-haspopup attribute on the ${prop} when overlayRole is set to alertdialog`, async () => {
           popover.overlayRole = 'alertdialog';
           await nextUpdate(popover);
           expect(element.getAttribute('aria-haspopup')).to.equal('dialog');
         });
 
+        it(`should update aria-haspopup attribute on the ${prop} when role is set to different value`, async () => {
+          popover.role = 'menu';
+          await nextUpdate(popover);
+          expect(element.getAttribute('aria-haspopup')).to.equal('true');
+        });
+
         it(`should update aria-haspopup attribute on the ${prop} when overlayRole is set to different value`, async () => {
           popover.overlayRole = 'menu';
           await nextUpdate(popover);