From 29e8821fdace89e8649aac51ec2259ca52c06f72 Mon Sep 17 00:00:00 2001 From: Martin Hristov Date: Mon, 15 Apr 2019 15:25:52 +0300 Subject: [PATCH 1/2] fix(ui5-button): removes active state after pressing tab on an active button - The activate state of the buttons used to be displayed and controlled by the :active CSS selector. Seems like :active has some issues on different browsers and it is not consistently implemented. Therefore, the active state is now controlled by a javascript implementation. - chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=945854 fixes: https://github.com/SAP/ui5-webcomponents/issues/156 --- packages/main/src/Button.js | 24 +++++++++++--- packages/main/src/ToggleButton.js | 10 ------ .../main/src/ToggleButtonTemplateContext.js | 1 - packages/main/src/themes-next/Button.css | 31 +++++++------------ 4 files changed, 31 insertions(+), 35 deletions(-) diff --git a/packages/main/src/Button.js b/packages/main/src/Button.js index 715f8efdc245..226590d8014f 100644 --- a/packages/main/src/Button.js +++ b/packages/main/src/Button.js @@ -160,6 +160,16 @@ class Button extends WebComponent { return ButtonTemplateContext.calculate; } + constructor() { + super(); + + this._deactivate = () => { + if (this._active) { + this._active = false; + } + } + } + onBeforeRendering() { if (this.icon) { this._iconSettings = { @@ -170,6 +180,14 @@ class Button extends WebComponent { } } + onEnterDOM() { + document.addEventListener("mouseup", this._deactivate); + } + + onExitDOM() { + document.removeEventListener("mouseup", this._deactivate); + } + onclick(event) { event.isMarked = "button"; if (!this.disabled) { @@ -179,16 +197,14 @@ class Button extends WebComponent { onmousedown(event) { event.isMarked = "button"; - if (this.activeIcon) { + + if (!this.disabled) { this._active = true; } } onmouseup(event) { event.isMarked = "button"; - if (this.activeIcon) { - this._active = false; - } } onkeydown(event) { diff --git a/packages/main/src/ToggleButton.js b/packages/main/src/ToggleButton.js index e8dd3803022c..d00c0a55c07d 100644 --- a/packages/main/src/ToggleButton.js +++ b/packages/main/src/ToggleButton.js @@ -81,16 +81,6 @@ class ToggleButton extends Button { } } - /* - * @override - */ - onkeydown() {} - - /* - * @override - */ - onkeyup() {} - static get calculateTemplateContext() { return ToggleButtonTemplateContext.calculate; } diff --git a/packages/main/src/ToggleButtonTemplateContext.js b/packages/main/src/ToggleButtonTemplateContext.js index 9015a693b3ac..e5c9e0d79588 100644 --- a/packages/main/src/ToggleButtonTemplateContext.js +++ b/packages/main/src/ToggleButtonTemplateContext.js @@ -5,7 +5,6 @@ class ToggleButtonTemplateContext { const calculatedState = ButtonTemplateContext.calculate(state); calculatedState.classes.main.sapMToggleBtnPressed = state.pressed; - calculatedState.classes.main.sapMBtnActive = false; calculatedState.pressed = state.pressed ? "true" : undefined; return calculatedState; diff --git a/packages/main/src/themes-next/Button.css b/packages/main/src/themes-next/Button.css index f0f4ec62869b..104a34f1c4e4 100644 --- a/packages/main/src/themes-next/Button.css +++ b/packages/main/src/themes-next/Button.css @@ -66,7 +66,7 @@ ui5-button span[data-sap-ui-wc-root] .sapMBtn::before { position: relative; } -.sapMBtn:hover { +.sapMBtn:not(.sapMBtnActive):hover { background: var(--sapUiButtonHoverBackground); } @@ -116,8 +116,7 @@ ui5-button span[data-sap-ui-wc-root] .sapMBtn::before { border: 0; } -.sapMBtnActive, -.sapMBtn:active { +.sapMBtnActive { background-image: none; background-color: var(--sapUiButtonActiveBackground); border-color: var(--_ui5_button_active_border_color); @@ -125,8 +124,7 @@ ui5-button span[data-sap-ui-wc-root] .sapMBtn::before { text-shadow: none; } -.sapMBtnActive:focus::after, -.sapMBtn:active:focus::after { +.sapMBtnActive:focus::after { border-color: var(--sapUiContentContrastFocusColor); } @@ -142,8 +140,7 @@ ui5-button span[data-sap-ui-wc-root] .sapMBtn::before { border-color: var(--_ui5_button_positive_border_hover_color); } -.sapMBtn.sapMBtnPositive.sapMBtnActive, -.sapMBtn.sapMBtnPositive:active { +.sapMBtn.sapMBtnPositive.sapMBtnActive { background-color: var(--sapUiButtonAcceptActiveBackground); border-color: var(--_ui5_button_positive_border_active_color); color: var(--sapUiButtonActiveTextColor); @@ -153,8 +150,7 @@ ui5-button span[data-sap-ui-wc-root] .sapMBtn::before { .sapMBtn.sapMBtnPositive:focus { border-color: var(--_ui5_button_positive_focus_border_color); } -.sapMBtn.sapMBtnPositive.sapMBtnActive:focus::after, -.sapMBtn.sapMBtnPositive:active:focus::after { +.sapMBtn.sapMBtnPositive.sapMBtnActive:focus::after { border-color: var(--sapUiContentContrastFocusColor); } @@ -178,16 +174,14 @@ ui5-button span[data-sap-ui-wc-root] .sapMBtn::before { border-color: var(--_ui5_button_negative_focus_border_color); } -.sapMBtn.sapMBtnNegative.sapMBtnActive, -.sapMBtn.sapMBtnNegative:active { +.sapMBtn.sapMBtnNegative.sapMBtnActive { background-color: var(--sapUiButtonRejectActiveBackground); border-color: var(--_ui5_button_negative_active_border_color); color: var(--sapUiButtonActiveTextColor); text-shadow: none; } -.sapMBtn.sapMBtnNegative.sapMBtnActive:focus::after, -.sapMBtn.sapMBtnNegative:active:focus::after { +.sapMBtn.sapMBtnNegative.sapMBtnActive:focus::after { border-color: var(--sapUiContentContrastFocusColor); } @@ -207,16 +201,14 @@ ui5-button span[data-sap-ui-wc-root] .sapMBtn::before { border-color: var(--sapUiButtonEmphasizedHoverBorderColor); } -.sapMBtn.sapMBtnEmphasized.sapMBtnActive, -.sapMBtn.sapMBtnEmphasized:active { +.sapMBtn.sapMBtnEmphasized.sapMBtnActive { background-color: var(--sapUiButtonEmphasizedActiveBackground); border-color: var(--sapUiButtonEmphasizedActiveBorderColor); color: var(--sapUiButtonActiveTextColor); text-shadow: none; } -.sapMBtn.sapMBtnEmphasized.sapMBtnActive:focus::after, -.sapMBtn.sapMBtnEmphasized:active:focus::after { +.sapMBtn.sapMBtnEmphasized.sapMBtnActive:focus::after { border-color: var(--sapUiContentContrastFocusColor); } @@ -240,13 +232,12 @@ ui5-button span[data-sap-ui-wc-root] .sapMBtn::before { background-color: var(--sapUiButtonLiteHoverBackground); } -.sapMBtn.sapMBtnTransparent.sapMBtnActive, -.sapMBtn.sapMBtnTransparent:active { +.sapMBtn.sapMBtnTransparent.sapMBtnActive { background-color: var(--sapUiButtonLiteActiveBackground); color: var(--sapUiButtonActiveTextColor); text-shadow: none; } -.sapMBtn.sapMBtnTransparent:hover:not(:active) { +.sapMBtn.sapMBtnTransparent:hover:not(.sapMBtnActive) { border-color: transparent; } From 2ad1a23f33318ef28b7dcaa299037baf9c6fefb4 Mon Sep 17 00:00:00 2001 From: Martin Hristov Date: Mon, 15 Apr 2019 15:35:17 +0300 Subject: [PATCH 2/2] fix eslint errors --- packages/main/src/Button.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/main/src/Button.js b/packages/main/src/Button.js index 226590d8014f..ca35702f4d84 100644 --- a/packages/main/src/Button.js +++ b/packages/main/src/Button.js @@ -167,7 +167,7 @@ class Button extends WebComponent { if (this._active) { this._active = false; } - } + }; } onBeforeRendering() { @@ -183,7 +183,7 @@ class Button extends WebComponent { onEnterDOM() { document.addEventListener("mouseup", this._deactivate); } - + onExitDOM() { document.removeEventListener("mouseup", this._deactivate); }