From 27bc1587082df55ffaec7571855e998bc9948f04 Mon Sep 17 00:00:00 2001 From: Thomas Marstrander Date: Mon, 23 May 2016 14:34:56 +0200 Subject: [PATCH 1/3] Use buttons instead of anchors in Confirmation Dialog. HFJ-1930 --- js/h5p-confirmation-dialog.js | 71 ++++++++++++------------------ styles/h5p-confirmation-dialog.css | 10 ++--- styles/h5p-core-button.css | 25 ++++++----- 3 files changed, 47 insertions(+), 59 deletions(-) diff --git a/js/h5p-confirmation-dialog.js b/js/h5p-confirmation-dialog.js index a8d4ac2..11047b2 100644 --- a/js/h5p-confirmation-dialog.js +++ b/js/h5p-confirmation-dialog.js @@ -24,6 +24,26 @@ H5P.ConfirmationDialog = (function (EventDispatcher) { options.cancelText = options.cancelText || H5P.t('cancelLabel'); options.confirmText = options.confirmText || H5P.t('confirmLabel'); + /** + * Handle confirming event + * @param {Event} e + */ + function dialogConfirmed(e) { + self.hide(); + self.trigger('confirmed'); + e.preventDefault(); + } + + /** + * Handle dialog canceled + * @param {Event} e + */ + function dialogCanceled(e) { + self.hide(); + self.trigger('canceled'); + e.preventDefault(); + } + // Offset of exit button var exitButtonOffset = 2 * 16; var shadowOffset = 8; @@ -69,70 +89,37 @@ H5P.ConfirmationDialog = (function (EventDispatcher) { body.appendChild(buttons); // Cancel button - var cancelButton = document.createElement('a'); + var cancelButton = document.createElement('button'); cancelButton.classList.add('h5p-core-cancel-button'); - cancelButton.href = '#'; cancelButton.textContent = options.cancelText; - cancelButton.onclick = function (e) { - self.hide(); - self.trigger('canceled'); - e.preventDefault(); - }; + cancelButton.onclick = dialogCanceled; cancelButton.onkeydown = function (e) { if (e.which === 32) { // Space - // Prevent jumping - e.preventDefault(); - } - else if (e.which === 13) { // Enter - self.hide(); - self.trigger('canceled'); - e.preventDefault(); + dialogCanceled(e); } }; buttons.appendChild(cancelButton); // Confirm button - var confirmButton = document.createElement('a'); + var confirmButton = document.createElement('button'); confirmButton.classList.add('h5p-core-button', 'h5p-confirmation-dialog-confirm-button'); - confirmButton.href = '#'; confirmButton.textContent = options.confirmText; - confirmButton.onclick = function (e) { - self.hide(); - self.trigger('confirmed'); - e.preventDefault(); - }; + confirmButton.onclick = dialogConfirmed; confirmButton.onkeydown = function (e) { if (e.which === 32) { // Space - // Prevent jumping - e.preventDefault(); - } - else if (e.which === 13) { // Enter - self.hide(); - self.trigger('confirmed'); - e.preventDefault(); + dialogConfirmed(e); } }; buttons.appendChild(confirmButton); // Exit button - var exitButton = document.createElement('a'); - exitButton.href = '#'; + var exitButton = document.createElement('button'); exitButton.classList.add('h5p-confirmation-dialog-exit'); - exitButton.onclick = function (e) { - self.hide(); - self.trigger('canceled'); - e.preventDefault(); - }; + exitButton.onclick = dialogCanceled; exitButton.onkeydown = function (e) { if (e.which === 32) { // Space - // Prevent jumping - e.preventDefault(); - } - else if (e.which === 13) { // Enter - self.hide(); - self.trigger('canceled'); - e.preventDefault(); + dialogCanceled(e); } }; popup.appendChild(exitButton); diff --git a/styles/h5p-confirmation-dialog.css b/styles/h5p-confirmation-dialog.css index 093a6ad..2923b17 100644 --- a/styles/h5p-confirmation-dialog.css +++ b/styles/h5p-confirmation-dialog.css @@ -81,9 +81,9 @@ float: right; } -a.h5p-confirmation-dialog-exit:visited, -a.h5p-confirmation-dialog-exit:link, -a.h5p-confirmation-dialog-exit { +button.h5p-confirmation-dialog-exit:visited, +button.h5p-confirmation-dialog-exit:link, +button.h5p-confirmation-dialog-exit { position: absolute; background: none; border: none; @@ -95,8 +95,8 @@ a.h5p-confirmation-dialog-exit { text-decoration: none; } -a.h5p-confirmation-dialog-exit:focus, -a.h5p-confirmation-dialog-exit:hover { +button.h5p-confirmation-dialog-exit:focus, +button.h5p-confirmation-dialog-exit:hover { color: #555; } diff --git a/styles/h5p-core-button.css b/styles/h5p-core-button.css index b05c423..04f33a3 100644 --- a/styles/h5p-core-button.css +++ b/styles/h5p-core-button.css @@ -1,6 +1,6 @@ -a.h5p-core-button:visited, -a.h5p-core-button:link, -a.h5p-core-button { +button.h5p-core-button:visited, +button.h5p-core-button:link, +button.h5p-core-button { font-size: 1em; line-height: 1.2; padding: 0.5em 1.25em; @@ -20,15 +20,15 @@ a.h5p-core-button { vertical-align: baseline; text-decoration: none; } -a.h5p-core-button:hover, -a.h5p-core-button:focus { +button.h5p-core-button:hover, +button.h5p-core-button:focus { background: #1356a3; color: #fff; text-decoration: none; -webkit-transition: initial; transition: initial; } -a.h5p-core-button:active { +button.h5p-core-button:active { position: relative; background: #104888; @@ -37,7 +37,7 @@ a.h5p-core-button:active { box-shadow: inset 0 4px 0 #0e407a; } -a.h5p-core-button:before { +button.h5p-core-button:before { font-family: 'H5P'; padding-right: 0.15em; font-size: 1.5em; @@ -45,9 +45,10 @@ a.h5p-core-button:before { line-height: 0.7; } -a.h5p-core-cancel-button:visited, -a.h5p-core-cancel-button:link, -a.h5p-core-cancel-button { +button.h5p-core-cancel-button:visited, +button.h5p-core-cancel-button:link, +button.h5p-core-cancel-button { + border: none; background: none; color: #a00; margin-right: 1em; @@ -55,8 +56,8 @@ a.h5p-core-cancel-button { text-decoration: none; } -a.h5p-core-cancel-button:hover, -a.h5p-core-cancel-button:focus { +button.h5p-core-cancel-button:hover, +button.h5p-core-cancel-button:focus { background: none; border: none; color: #e40000; From c6b3b06a97f3e30b8cbc0d16ef31fcb7559e95bd Mon Sep 17 00:00:00 2001 From: Thomas Marstrander Date: Tue, 24 May 2016 11:18:42 +0200 Subject: [PATCH 2/3] Added pointer cursor for core cancel button. HFJ-1930 --- styles/h5p-core-button.css | 1 + 1 file changed, 1 insertion(+) diff --git a/styles/h5p-core-button.css b/styles/h5p-core-button.css index 04f33a3..3ba392e 100644 --- a/styles/h5p-core-button.css +++ b/styles/h5p-core-button.css @@ -54,6 +54,7 @@ button.h5p-core-cancel-button { margin-right: 1em; font-size: 1em; text-decoration: none; + cursor: pointer; } button.h5p-core-cancel-button:hover, From 0a63d123cf6e3865d6c72fefa819084cdcd21fbc Mon Sep 17 00:00:00 2001 From: Thomas Marstrander Date: Tue, 31 May 2016 10:19:07 +0200 Subject: [PATCH 3/3] Prevent DOM pollution when having many confirmation dialogs. Append confirmation dialog on show and detach it on hide. HFJ-1969 --- js/h5p-confirmation-dialog.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/h5p-confirmation-dialog.js b/js/h5p-confirmation-dialog.js index 11047b2..c948192 100644 --- a/js/h5p-confirmation-dialog.js +++ b/js/h5p-confirmation-dialog.js @@ -128,14 +128,12 @@ H5P.ConfirmationDialog = (function (EventDispatcher) { var wrapperElement; /** - * Append confirmation dialog + * Set parent of confirmation dialog * @param {HTMLElement} wrapper * @returns {H5P.ConfirmationDialog} */ this.appendTo = function (wrapper) { - wrapper.appendChild(popupBackground); wrapperElement = wrapper; - return this; }; @@ -169,6 +167,7 @@ H5P.ConfirmationDialog = (function (EventDispatcher) { * @returns {H5P.ConfirmationDialog} */ this.show = function (offsetTop) { + wrapperElement.appendChild(popupBackground); popupBackground.classList.remove('hidden'); fitToContainer(offsetTop); setTimeout(function () { @@ -202,6 +201,7 @@ H5P.ConfirmationDialog = (function (EventDispatcher) { popup.classList.add('hidden'); setTimeout(function () { popupBackground.classList.add('hidden'); + wrapperElement.removeChild(popupBackground); }, 100); return this;