From ab20a40dd169370b871c2656c1939d3d9341e442 Mon Sep 17 00:00:00 2001 From: web-padawan Date: Thu, 8 Jun 2023 14:54:25 +0300 Subject: [PATCH] fix!: only modify text content for default slotted buttons --- packages/crud/src/vaadin-crud.js | 94 ++++++----- packages/crud/test/crud-buttons.test.js | 204 +++++++++++++++++++----- 2 files changed, 224 insertions(+), 74 deletions(-) diff --git a/packages/crud/src/vaadin-crud.js b/packages/crud/src/vaadin-crud.js index 0d5d505fa41..0773dc02f45 100644 --- a/packages/crud/src/vaadin-crud.js +++ b/packages/crud/src/vaadin-crud.js @@ -25,16 +25,48 @@ import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mix import { getProperty, setProperty } from './vaadin-crud-helpers.js'; /** + * A controller for initializing slotted button. * @private - * A to initialize slotted button. */ class ButtonSlotController extends SlotController { constructor(host, type, theme) { - super(host, `${type}-button`, 'vaadin-button', { - initializer: (button) => { - button.setAttribute('theme', theme); - }, - }); + super(host, `${type}-button`, 'vaadin-button'); + + this.type = type; + this.theme = theme; + } + + /** + * Override method inherited from `SlotController` + * to mark custom slotted button as the default. + * + * @param {Node} node + * @protected + * @override + */ + initNode(node) { + // Needed by Flow counterpart to apply i18n to custom button + if (node._isDefault) { + this.defaultNode = node; + } + + if (node === this.defaultNode) { + node.setAttribute('theme', this.theme); + } + + const { host, type } = this; + const property = `_${type}Button`; + const listener = `__${type}`; + + // TODO: restore default button when a corresponding slotted button is removed. + // This would require us to detect where to insert a button after teleporting it, + // which happens after opening a dialog and closing it (default editor position). + if (host[property] && host[property] !== node) { + host[property].remove(); + } + + node.addEventListener('click', host[listener]); + host[property] = node; } } @@ -664,6 +696,11 @@ class Crud extends ControllerMixin(ElementMixin(ThemableMixin(PolymerElement))) this.__onGridSizeChanged = this.__onGridSizeChanged.bind(this); this.__onGridActiveItemChanged = this.__onGridActiveItemChanged.bind(this); + this._newButtonController = new ButtonSlotController(this, 'new', 'primary'); + this._saveButtonController = new ButtonSlotController(this, 'save', 'primary'); + this._cancelButtonController = new ButtonSlotController(this, 'cancel', 'tertiary'); + this._deleteButtonController = new ButtonSlotController(this, 'delete', 'tertiary error'); + this._observer = new FlattenedNodesObserver(this, (info) => { this.__onDomChange(info.addedNodes); }); @@ -705,12 +742,12 @@ class Crud extends ControllerMixin(ElementMixin(ThemableMixin(PolymerElement))) this.addController(new SlotController(this, 'form', 'vaadin-crud-form')); - this.addController(new ButtonSlotController(this, 'new', 'primary')); + this.addController(this._newButtonController); // NOTE: order in which buttons are added should match the order of slots in template - this.addController(new ButtonSlotController(this, 'save', 'primary')); - this.addController(new ButtonSlotController(this, 'cancel', 'tertiary')); - this.addController(new ButtonSlotController(this, 'delete', 'tertiary error')); + this.addController(this._saveButtonController); + this.addController(this._cancelButtonController); + this.addController(this._deleteButtonController); this.addController( new MediaQueryController(this._fullscreenMediaQuery, (matches) => { @@ -863,9 +900,6 @@ class Crud extends ControllerMixin(ElementMixin(ThemableMixin(PolymerElement))) /** @private */ __onDomChange(addedNodes) { - // TODO: restore default button when a corresponding slotted button is removed. - // This would require us to detect where to insert a button after teleporting it, - // which happens after opening a dialog and closing it (default editor position). addedNodes .filter((node) => node.nodeType === Node.ELEMENT_NODE) .forEach((node) => { @@ -881,9 +915,6 @@ class Crud extends ControllerMixin(ElementMixin(ThemableMixin(PolymerElement))) this.__editOnClickChanged(this.editOnClick, this._grid); } else if (slotAttributeValue === 'form') { this._form = node; - } else if (slotAttributeValue.indexOf('button') >= 0) { - const [button] = slotAttributeValue.split('-'); - this.__setupSlottedButton(button, node); } }); @@ -1016,7 +1047,10 @@ class Crud extends ControllerMixin(ElementMixin(ThemableMixin(PolymerElement))) __saveButtonPropsChanged(saveButton, i18nLabel, isDirty) { if (saveButton) { saveButton.toggleAttribute('disabled', this.__isSaveBtnDisabled(isDirty)); - saveButton.textContent = i18nLabel; + + if (saveButton === this._saveButtonController.defaultNode) { + saveButton.textContent = i18nLabel; + } } } @@ -1028,8 +1062,11 @@ class Crud extends ControllerMixin(ElementMixin(ThemableMixin(PolymerElement))) */ __deleteButtonPropsChanged(deleteButton, i18nLabel, isNew) { if (deleteButton) { - deleteButton.textContent = i18nLabel; deleteButton.toggleAttribute('hidden', isNew); + + if (deleteButton === this._deleteButtonController.defaultNode) { + deleteButton.textContent = i18nLabel; + } } } @@ -1039,7 +1076,7 @@ class Crud extends ControllerMixin(ElementMixin(ThemableMixin(PolymerElement))) * @private */ __cancelButtonPropsChanged(cancelButton, i18nLabel) { - if (cancelButton) { + if (cancelButton && cancelButton === this._cancelButtonController.defaultNode) { cancelButton.textContent = i18nLabel; } } @@ -1050,28 +1087,11 @@ class Crud extends ControllerMixin(ElementMixin(ThemableMixin(PolymerElement))) * @private */ __newButtonPropsChanged(newButton, i18nNewItem) { - if (newButton) { + if (newButton && newButton === this._newButtonController.defaultNode) { newButton.textContent = i18nNewItem; } } - /** - * @param {string} type - * @param {HTMLElement} newButton - * @private - */ - __setupSlottedButton(type, button) { - const property = `_${type}Button`; - const listener = `__${type}`; - - if (this[property] && this[property] !== button) { - this[property].remove(); - } - - button.addEventListener('click', this[listener]); - this[property] = button; - } - /** @private */ __dataProviderChanged(dataProvider) { if (this._grid) { diff --git a/packages/crud/test/crud-buttons.test.js b/packages/crud/test/crud-buttons.test.js index 464b920b80d..8a44cdc0603 100644 --- a/packages/crud/test/crud-buttons.test.js +++ b/packages/crud/test/crud-buttons.test.js @@ -12,9 +12,11 @@ describe('crud buttons', () => { } ['default', 'slotted'].forEach((mode) => { + const isDefault = mode === 'default'; + describe(mode, () => { beforeEach(async () => { - if (mode === 'default') { + if (isDefault) { crud = fixtureSync(''); } else { crud = fixtureSync(` @@ -33,31 +35,33 @@ describe('crud buttons', () => { }); describe('i18n', () => { - it('should set the label for the delete button', () => { - expect(deleteButton.textContent).to.equal(crud.i18n.deleteItem); - }); + (isDefault ? describe : describe.skip)('i18n', () => { + it('should set the label for the delete button', () => { + expect(deleteButton.textContent).to.equal(crud.i18n.deleteItem); + }); - it('should update the label of the delete button on i18n property change', () => { - crud.i18n = { ...crud.i18n, deleteItem: 'Custom' }; - expect(deleteButton.textContent).to.equal('Custom'); - }); + it('should update the label of the delete button on i18n property change', () => { + crud.i18n = { ...crud.i18n, deleteItem: 'Custom' }; + expect(deleteButton.textContent).to.equal('Custom'); + }); - it('should set the label for the save button', () => { - expect(saveButton.textContent).to.equal(crud.i18n.saveItem); - }); + it('should set the label for the save button', () => { + expect(saveButton.textContent).to.equal(crud.i18n.saveItem); + }); - it('should update the label of the save button on i18n property change', () => { - crud.i18n = { ...crud.i18n, saveItem: 'Custom' }; - expect(saveButton.textContent).to.equal('Custom'); - }); + it('should update the label of the save button on i18n property change', () => { + crud.i18n = { ...crud.i18n, saveItem: 'Custom' }; + expect(saveButton.textContent).to.equal('Custom'); + }); - it('should set the label for the cancel button', () => { - expect(cancelButton.textContent).to.equal(crud.i18n.cancel); - }); + it('should set the label for the cancel button', () => { + expect(cancelButton.textContent).to.equal(crud.i18n.cancel); + }); - it('should update the label of the cancel button on i18n property change', () => { - crud.i18n = { ...crud.i18n, cancel: 'Custom' }; - expect(cancelButton.textContent).to.equal('Custom'); + it('should update the label of the cancel button on i18n property change', () => { + crud.i18n = { ...crud.i18n, cancel: 'Custom' }; + expect(cancelButton.textContent).to.equal('Custom'); + }); }); }); @@ -687,30 +691,156 @@ describe('crud buttons', () => { }); describe('lazy', () => { + let button; + beforeEach(async () => { crud = fixtureSync(''); - await nextRender(crud); + await nextRender(); + button = document.createElement('vaadin-button'); }); - it('should set the label for the delete button when it is added lazily', async () => { - const newButton = fixtureSync(``); - crud.appendChild(newButton); - await nextRender(crud); - expect(newButton.textContent).to.equal(crud.i18n.deleteItem); + describe('new', () => { + beforeEach(() => { + button.setAttribute('slot', 'new-button'); + }); + + it('should not change text content of the custom new item button added lazily', async () => { + button.textContent = 'New user'; + + crud.appendChild(button); + await nextRender(); + + expect(button.textContent).to.equal('New user'); + }); + + it('should set text content of the new item button when marked as a default', async () => { + button._isDefault = true; + + crud.appendChild(button); + await nextRender(); + + expect(button.textContent).to.equal(crud.i18n.newItem); + }); + + it('should update the new item button marked as default on i18n property change', async () => { + button._isDefault = true; + + crud.appendChild(button); + await nextRender(); + + crud.i18n = { ...crud.i18n, newItem: 'Add user' }; + await nextRender(); + + expect(button.textContent).to.equal('Add user'); + }); }); - it('should set the label for the save button when it is added lazily', async () => { - const newButton = fixtureSync(``); - crud.appendChild(newButton); - await nextRender(crud); - expect(newButton.textContent).to.equal(crud.i18n.saveItem); + describe('delete', () => { + beforeEach(() => { + button.setAttribute('slot', 'delete-button'); + }); + + it('should not change text content of the custom delete button added lazily', async () => { + button.textContent = 'Drop user'; + + crud.appendChild(button); + await nextRender(); + + expect(button.textContent).to.equal('Drop user'); + }); + + it('should set text content of the delete button when marked as a default', async () => { + button._isDefault = true; + + crud.appendChild(button); + await nextRender(); + + expect(button.textContent).to.equal(crud.i18n.deleteItem); + }); + + it('should update the delete button marked as default on i18n property change', async () => { + button._isDefault = true; + + crud.appendChild(button); + await nextRender(); + + crud.i18n = { ...crud.i18n, deleteItem: 'Drop user' }; + await nextRender(); + + expect(button.textContent).to.equal('Drop user'); + }); }); - it('should set the label for the cancel button when it is added lazily', async () => { - const newButton = fixtureSync(``); - crud.appendChild(newButton); - await nextRender(crud); - expect(newButton.textContent).to.equal(crud.i18n.cancel); + describe('save', () => { + beforeEach(() => { + button.setAttribute('slot', 'save-button'); + }); + + it('should not set text content for the custom save button added lazily', async () => { + button.textContent = 'Save user'; + + crud.appendChild(button); + await nextRender(); + + expect(button.textContent).to.equal('Save user'); + }); + + it('should set text content for the save button when marked as the default', async () => { + button._isDefault = true; + + crud.appendChild(button); + await nextRender(); + + expect(button.textContent).to.equal(crud.i18n.saveItem); + }); + + it('should update the save button marked as default on i18n property change', async () => { + button._isDefault = true; + + crud.appendChild(button); + await nextRender(); + + crud.i18n = { ...crud.i18n, saveItem: 'Save user' }; + await nextRender(); + + expect(button.textContent).to.equal('Save user'); + }); + }); + + describe('cancel', () => { + beforeEach(() => { + button.setAttribute('slot', 'cancel-button'); + }); + + it('should not set text content for the custom cancel button added lazily', async () => { + button.textContent = 'Discard'; + + crud.appendChild(button); + await nextRender(); + + expect(button.textContent).to.equal('Discard'); + }); + + it('should set text content for the cancel button when marked as the default', async () => { + button._isDefault = true; + + crud.appendChild(button); + await nextRender(); + + expect(button.textContent).to.equal(crud.i18n.cancel); + }); + + it('should update the cancel button marked as default on i18n property change', async () => { + button._isDefault = true; + + crud.appendChild(button); + await nextRender(); + + crud.i18n = { ...crud.i18n, cancel: 'Discard' }; + await nextRender(); + + expect(button.textContent).to.equal('Discard'); + }); }); });