From 2b244133e1e5b855a8c3a5ee07fefb5e990dfd9c Mon Sep 17 00:00:00 2001 From: Alex P Date: Tue, 14 Feb 2023 17:12:08 +0200 Subject: [PATCH] Make error message list more consistent with WC Now it should match the behavior of WC checkout.js more closely and avoid duplicated message lists (because we were always creating our own list ignoring WC). Also removed the persist parameter. It does not seem to be used for anything useful, and we only set it inconsistently for some of the errors (remained from some old code). I guess the idea could be to prevent genericError() from clearing previously added errors. Such if you call clear(), added some messages, but then some generic handler also triggered and called genericError(). If we really need such behavior, then we can simply check whether the list is empty. But it's probably not a good idea anyway because it can cause confusion if genericError() was a result of some other operation unrelated to the previous errors (e.g. tried another payment gateway). --- modules/ppcp-button/resources/js/button.js | 5 +- .../ActionHandler/CheckoutActionHandler.js | 4 +- .../resources/js/modules/ErrorHandler.js | 69 +++++++------------ .../js/modules/Renderer/CreditCardRenderer.js | 8 +-- 4 files changed, 36 insertions(+), 50 deletions(-) diff --git a/modules/ppcp-button/resources/js/button.js b/modules/ppcp-button/resources/js/button.js index 8dc519a6c..945003bd5 100644 --- a/modules/ppcp-button/resources/js/button.js +++ b/modules/ppcp-button/resources/js/button.js @@ -28,7 +28,10 @@ const cardsSpinner = new Spinner('#ppcp-hosted-fields'); const bootstrap = () => { const checkoutFormSelector = 'form.woocommerce-checkout'; - const errorHandler = new ErrorHandler(PayPalCommerceGateway.labels.error.generic); + const errorHandler = new ErrorHandler( + PayPalCommerceGateway.labels.error.generic, + document.querySelector(checkoutFormSelector) ?? document.querySelector('.woocommerce-notices-wrapper') + ); const spinner = new Spinner(); const creditCardRenderer = new CreditCardRenderer(PayPalCommerceGateway, errorHandler, spinner); diff --git a/modules/ppcp-button/resources/js/modules/ActionHandler/CheckoutActionHandler.js b/modules/ppcp-button/resources/js/modules/ActionHandler/CheckoutActionHandler.js index e743f2083..6f28e8a2f 100644 --- a/modules/ppcp-button/resources/js/modules/ActionHandler/CheckoutActionHandler.js +++ b/modules/ppcp-button/resources/js/modules/ActionHandler/CheckoutActionHandler.js @@ -62,9 +62,9 @@ class CheckoutActionHandler { if (data.data.errors?.length > 0) { errorHandler.messages(data.data.errors); } else if (data.data.details?.length > 0) { - errorHandler.message(data.data.details.map(d => `${d.issue} ${d.description}`).join('
'), true); + errorHandler.message(data.data.details.map(d => `${d.issue} ${d.description}`).join('
')); } else { - errorHandler.message(data.data.message, true); + errorHandler.message(data.data.message); } } diff --git a/modules/ppcp-button/resources/js/modules/ErrorHandler.js b/modules/ppcp-button/resources/js/modules/ErrorHandler.js index c402aa751..726ef2074 100644 --- a/modules/ppcp-button/resources/js/modules/ErrorHandler.js +++ b/modules/ppcp-button/resources/js/modules/ErrorHandler.js @@ -1,47 +1,41 @@ class ErrorHandler { - constructor(genericErrorText) + /** + * @param {String} genericErrorText + * @param {Element} wrapper + */ + constructor(genericErrorText, wrapper) { this.genericErrorText = genericErrorText; - this.wrapper = document.querySelector('.woocommerce-notices-wrapper'); - this.messagesList = document.querySelector('ul.woocommerce-error'); + this.wrapper = wrapper; } genericError() { - if (this.wrapper.classList.contains('ppcp-persist')) { - return; - } this.clear(); this.message(this.genericErrorText) } appendPreparedErrorMessageElement(errorMessageElement) { - if (this.messagesList === null) { - this._prepareMessagesList(); - } - - this.messagesList.replaceWith(errorMessageElement); + this._getMessageContainer().replaceWith(errorMessageElement); } /** * @param {String} text - * @param {Boolean} persist */ - message(text, persist = false) + message(text) { - this._addMessage(text, persist); + this._addMessage(text); this._scrollToMessages(); } /** * @param {Array} texts - * @param {Boolean} persist */ - messages(texts, persist = false) + messages(texts) { - texts.forEach(t => this._addMessage(t, persist)); + texts.forEach(t => this._addMessage(t)); this._scrollToMessages(); } @@ -49,26 +43,17 @@ class ErrorHandler { /** * @private * @param {String} text - * @param {Boolean} persist */ - _addMessage(text, persist = false) + _addMessage(text) { if(! typeof String || text.length === 0) { throw new Error('A new message text must be a non-empty string.'); } - if (this.messagesList === null){ - this._prepareMessagesList(); - } + const messageContainer = this._getMessageContainer(); - if (persist) { - this.wrapper.classList.add('ppcp-persist'); - } else { - this.wrapper.classList.remove('ppcp-persist'); - } - - let messageNode = this._prepareMessagesListItem(text); - this.messagesList.appendChild(messageNode); + let messageNode = this._prepareMessageElement(text); + messageContainer.appendChild(messageNode); } /** @@ -76,26 +61,28 @@ class ErrorHandler { */ _scrollToMessages() { - jQuery.scroll_to_notices(jQuery('.woocommerce-notices-wrapper')); + jQuery.scroll_to_notices(jQuery('.woocommerce-error')); } /** * @private */ - _prepareMessagesList() + _getMessageContainer() { - if (this.messagesList === null) { - this.messagesList = document.createElement('ul'); - this.messagesList.setAttribute('class', 'woocommerce-error'); - this.messagesList.setAttribute('role', 'alert'); - this.wrapper.appendChild(this.messagesList); + let messageContainer = document.querySelector('ul.woocommerce-error'); + if (messageContainer === null) { + messageContainer = document.createElement('ul'); + messageContainer.setAttribute('class', 'woocommerce-error'); + messageContainer.setAttribute('role', 'alert'); + jQuery(this.wrapper).prepend(messageContainer); } + return messageContainer; } /** * @private */ - _prepareMessagesListItem(message) + _prepareMessageElement(message) { const li = document.createElement('li'); li.innerHTML = message; @@ -105,11 +92,7 @@ class ErrorHandler { clear() { - if (this.messagesList === null) { - return; - } - - this.messagesList.innerHTML = ''; + jQuery( '.woocommerce-error, .woocommerce-message' ).remove(); } } diff --git a/modules/ppcp-button/resources/js/modules/Renderer/CreditCardRenderer.js b/modules/ppcp-button/resources/js/modules/Renderer/CreditCardRenderer.js index 3eefb59ac..0ddafdd15 100644 --- a/modules/ppcp-button/resources/js/modules/Renderer/CreditCardRenderer.js +++ b/modules/ppcp-button/resources/js/modules/Renderer/CreditCardRenderer.js @@ -234,15 +234,15 @@ class CreditCardRenderer { this.errorHandler.clear(); if (err.data?.details?.length) { - this.errorHandler.message(err.data.details.map(d => `${d.issue} ${d.description}`).join('
'), true); + this.errorHandler.message(err.data.details.map(d => `${d.issue} ${d.description}`).join('
')); } else if (err.details?.length) { - this.errorHandler.message(err.details.map(d => `${d.issue} ${d.description}`).join('
'), true); + this.errorHandler.message(err.details.map(d => `${d.issue} ${d.description}`).join('
')); } else if (err.data?.errors?.length > 0) { this.errorHandler.messages(err.data.errors); } else if (err.data?.message) { - this.errorHandler.message(err.data.message, true); + this.errorHandler.message(err.data.message); } else if (err.message) { - this.errorHandler.message(err.message, true); + this.errorHandler.message(err.message); } else { this.errorHandler.genericError(); }