fix: set focus-ring attribute on new button (#2368)

packages/button/package.json

@@ -26,13 +26,14 @@
   "dependencies": {
     "@polymer/polymer": "^3.0.0",
     "@vaadin/field-base": "^22.0.0-alpha1",
-    "@vaadin/vaadin-lumo-styles": "^22.0.0-alpha1",
+    "@vaadin/vaadin-control-state-mixin": "^22.0.0-alpha1",
     "@vaadin/vaadin-element-mixin": "^22.0.0-alpha1",
+    "@vaadin/vaadin-lumo-styles": "^22.0.0-alpha1",
     "@vaadin/vaadin-material-styles": "^22.0.0-alpha1",
     "@vaadin/vaadin-themable-mixin": "^22.0.0-alpha1"
   },
   "devDependencies": {
-    "@esm-bundle/chai": "^4.1.5",
+    "@esm-bundle/chai": "^4.3.4",
     "@vaadin/testing-helpers": "^0.2.1",
     "sinon": "^9.2.4"
   },

packages/button/src/vaadin-button.d.ts

@@ -6,12 +6,11 @@
 import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
 import { ElementMixin } from '@vaadin/vaadin-element-mixin/vaadin-element-mixin.js';
 import { ActiveMixin } from '@vaadin/field-base/src/active-mixin.js';
-import { DisabledMixin } from '@vaadin/field-base/src/disabled-mixin.js';
-import { DelegateFocusMixin } from '@vaadin/field-base/src/delegate-focus-mixin.js';
+import { ControlStateMixin } from '@vaadin/vaadin-control-state-mixin/vaadin-control-state-mixin.js';
 
-declare class Button extends ActiveMixin(DelegateFocusMixin(DisabledMixin(ElementMixin(ThemableMixin(HTMLElement))))) {
+declare class Button extends ControlStateMixin(ActiveMixin(ElementMixin(ThemableMixin(HTMLElement)))) {
   /**
-   * A getter that returns the native button as a focusable element for DelegateFocusMixin.
+   * A getter that returns the native button as a focusable element for ControlStateMixin.
    */
   readonly focusElement: HTMLButtonElement | null;
 }

packages/button/src/vaadin-button.js

@@ -7,10 +7,9 @@ import { PolymerElement, html } from '@polymer/polymer/polymer-element.js';
 import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
 import { ElementMixin } from '@vaadin/vaadin-element-mixin/vaadin-element-mixin.js';
 import { ActiveMixin } from '@vaadin/field-base/src/active-mixin.js';
-import { DisabledMixin } from '@vaadin/field-base/src/disabled-mixin.js';
-import { DelegateFocusMixin } from '@vaadin/field-base/src/delegate-focus-mixin.js';
+import { ControlStateMixin } from '@vaadin/vaadin-control-state-mixin/vaadin-control-state-mixin.js';
 
-class Button extends ActiveMixin(DelegateFocusMixin(DisabledMixin(ElementMixin(ThemableMixin(PolymerElement))))) {
+class Button extends ControlStateMixin(ActiveMixin(ElementMixin(ThemableMixin(PolymerElement)))) {
   static get is() {
     return 'vaadin-button';
   }
@@ -81,7 +80,7 @@ class Button extends ActiveMixin(DelegateFocusMixin(DisabledMixin(ElementMixin(T
   }
 
   /**
-   * A getter that returns the native button as a focusable element for DelegateFocusMixin.
+   * A getter that returns the native button as a focusable element for ControlStateMixin.
    *
    * @protected
    * @override

packages/button/test/button.test.js

@@ -1,5 +1,5 @@
 import { expect } from '@esm-bundle/chai';
-import sinon from 'sinon';
+import { sendKeys } from '@web/test-runner-commands';
 import {
   arrowDownKeyDown,
   enterKeyDown,
@@ -47,7 +47,7 @@ describe('vaadin-button', () => {
 
     beforeEach(() => {
       element = fixtureSync('<vaadin-button>Vaadin <i>Button</i></vaadin-button>');
-      button = element.shadowRoot.querySelector('button');
+      button = element.$.button;
       label = element.shadowRoot.querySelector('[part=label]');
     });
 
@@ -61,21 +61,9 @@ describe('vaadin-button', () => {
       expect(children[2].outerHTML).to.be.equal('<i>Button</i>');
     });
 
-    describe('disabled', () => {
-      beforeEach(() => {
-        element.disabled = true;
-      });
-
-      it('should disable the native button', () => {
-        expect(button.hasAttribute('disabled')).to.be.eql(true);
-      });
-
-      it('should not fire the click event', () => {
-        const spy = sinon.spy();
-        element.addEventListener('click', spy);
-        element.click();
-        expect(spy.called).to.be.false;
-      });
+    it('should disable the native button', () => {
+      element.disabled = true;
+      expect(button.hasAttribute('disabled')).to.be.true;
     });
 
     // TODO: Remove when it will be possible to detect whether the button element inherits ActiveMixin.
@@ -150,5 +138,25 @@ describe('vaadin-button', () => {
         expect(element.hasAttribute('active')).to.be.false;
       });
     });
+
+    describe('focus-ring', () => {
+      beforeEach(async () => {
+        // Focus on the button
+        await sendKeys({ press: 'Tab' });
+      });
+
+      it('should set focus-ring attribute when focusing with Tab', () => {
+        expect(element.hasAttribute('focus-ring')).to.be.true;
+      });
+
+      it('should remove focus-ring attribute when loosing focus with Shift+Tab', async () => {
+        // Focus out of the button
+        await sendKeys({ down: 'Shift' });
+        await sendKeys({ press: 'Tab' });
+        await sendKeys({ up: 'Shift' });
+
+        expect(element.hasAttribute('focus-ring')).to.be.false;
+      });
+    });
   });
 });