From a27fdd390ee4af0bbce280cfc481accd8c207834 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 27 Jan 2022 20:26:05 -0500 Subject: [PATCH 01/11] [Live] Fixing bug where inputs would not re-render if the value changed --- src/LiveComponent/CHANGELOG.md | 5 +- .../assets/src/clone_html_element.ts | 8 +++ .../assets/src/live_controller.ts | 26 ++++++-- .../normalize_attributes_for_comparison.ts | 18 ++++++ .../assets/test/controller/action.test.ts | 7 +- .../assets/test/controller/child.test.ts | 3 + .../assets/test/controller/csrf.test.ts | 5 +- .../assets/test/controller/model.test.ts | 29 +-------- .../assets/test/controller/render.test.ts | 38 ++++++++++- ...ormalize_attributes_for_comparison.test.ts | 64 +++++++++++++++++++ src/LiveComponent/src/Resources/doc/index.rst | 14 ++++ 11 files changed, 176 insertions(+), 41 deletions(-) create mode 100644 src/LiveComponent/assets/src/clone_html_element.ts create mode 100644 src/LiveComponent/assets/src/normalize_attributes_for_comparison.ts create mode 100644 src/LiveComponent/assets/test/normalize_attributes_for_comparison.test.ts diff --git a/src/LiveComponent/CHANGELOG.md b/src/LiveComponent/CHANGELOG.md index 824a8167d96..352d892ee7b 100644 --- a/src/LiveComponent/CHANGELOG.md +++ b/src/LiveComponent/CHANGELOG.md @@ -2,10 +2,13 @@ ## 2.1.0 +- Added `data-live-ignore` attribute. If included in an element, that element + will not be updated on re-render. + - The Live Component AJAX endpoints now return HTML in all situations instead of JSON. -- Send live action arguments to backend +- Ability to send live action arguments to backend ## 2.0.0 diff --git a/src/LiveComponent/assets/src/clone_html_element.ts b/src/LiveComponent/assets/src/clone_html_element.ts new file mode 100644 index 00000000000..b4ea6b5eecc --- /dev/null +++ b/src/LiveComponent/assets/src/clone_html_element.ts @@ -0,0 +1,8 @@ +export function cloneHTMLElement(element: HTMLElement): HTMLElement { + const newElement = element.cloneNode(true); + if (!(newElement instanceof HTMLElement)) { + throw new Error('Could not clone element'); + } + + return newElement; +} diff --git a/src/LiveComponent/assets/src/live_controller.ts b/src/LiveComponent/assets/src/live_controller.ts index e240b35228e..e13768932bb 100644 --- a/src/LiveComponent/assets/src/live_controller.ts +++ b/src/LiveComponent/assets/src/live_controller.ts @@ -5,6 +5,8 @@ import { combineSpacedArray } from './string_utils'; import { buildFormData, buildSearchParams } from './http_data_helper'; import { setDeepData, doesDeepPropertyExist, normalizeModelName } from './set_deep_data'; import { haveRenderedValuesChanged } from './have_rendered_values_changed'; +import { normalizeAttributesForComparison } from './normalize_attributes_for_comparison'; +import { cloneHTMLElement } from './clone_html_element'; interface ElementLoadingDirectives { element: HTMLElement, @@ -197,11 +199,7 @@ export default class extends Controller { const model = element.dataset.model || element.getAttribute('name'); if (!model) { - const clonedElement = (element.cloneNode()); - // helps typescript know this is an HTMLElement - if (!(clonedElement instanceof HTMLElement)) { - throw new Error('cloneNode() produced incorrect type'); - } + const clonedElement = cloneHTMLElement(element); throw new Error(`The update() method could not be called for "${clonedElement.outerHTML}": the element must either have a "data-model" or "name" attribute set to the model name.`); } @@ -558,7 +556,18 @@ export default class extends Controller { onBeforeElUpdated: (fromEl, toEl) => { // https://github.com/patrick-steele-idem/morphdom#can-i-make-morphdom-blaze-through-the-dom-tree-even-faster-yes if (fromEl.isEqualNode(toEl)) { - return false + // the nodes are equal, but the "value" on some might differ + // lets try to quickly compare a bit more deeply + const normalizedFromEl = cloneHTMLElement(fromEl); + normalizeAttributesForComparison(normalizedFromEl); + + const normalizedToEl = cloneHTMLElement(toEl); + normalizeAttributesForComparison(normalizedToEl); + + if (normalizedFromEl.isEqualNode(normalizedToEl)) { + // don't bother updating + return false; + } } // avoid updating child components: they will handle themselves @@ -571,6 +580,11 @@ export default class extends Controller { return false; } + // look for data-live-ignore, and don't update + if (fromEl.hasAttribute('data-live-ignore')) { + return false; + } + return true; } }); diff --git a/src/LiveComponent/assets/src/normalize_attributes_for_comparison.ts b/src/LiveComponent/assets/src/normalize_attributes_for_comparison.ts new file mode 100644 index 00000000000..011de99400b --- /dev/null +++ b/src/LiveComponent/assets/src/normalize_attributes_for_comparison.ts @@ -0,0 +1,18 @@ +/** + * Updates an HTML node to represent its underlying data. + * + * For example, this finds the value property of each underlying node + * and sets that onto the value attribute. This is useful to compare + * if two nodes are identical. + */ +export function normalizeAttributesForComparison(element: HTMLElement): void { + if (element.value) { + element.setAttribute('value', element.value); + } else if (element.hasAttribute('value')) { + element.setAttribute('value', ''); + } + + Array.from(element.children).forEach((child: HTMLElement) => { + normalizeAttributesForComparison(child); + }); +} diff --git a/src/LiveComponent/assets/test/controller/action.test.ts b/src/LiveComponent/assets/test/controller/action.test.ts index 7f64a798a4e..d2a00ff1fe0 100644 --- a/src/LiveComponent/assets/test/controller/action.test.ts +++ b/src/LiveComponent/assets/test/controller/action.test.ts @@ -42,6 +42,9 @@ describe('LiveController Action Tests', () => { afterEach(() => { clearDOM(); + if (!fetchMock.done()) { + throw new Error('Mocked requests did not match'); + } fetchMock.reset(); }); @@ -62,8 +65,6 @@ describe('LiveController Action Tests', () => { await waitFor(() => expect(element).toHaveTextContent('Comment Saved!')); expect(getByLabelText(element, 'Comments:')).toHaveValue('hi weaver'); - fetchMock.done(); - expect(postMock.lastOptions().body.get('comments')).toEqual('hi WEAVER'); }); @@ -71,7 +72,7 @@ describe('LiveController Action Tests', () => { const data = { comments: 'hi' }; const { element } = await startStimulus(template(data)); - fetchMock.postOnce('http://localhost/_components/my_component/sendNamedArgs?values=a%3D1%26b%3D2%26c%3D3', { + fetchMock.postOnce('http://localhost/_components/my_component/sendNamedArgs?args=a%3D1%26b%3D2%26c%3D3', { html: template({ comments: 'hi' }), }); diff --git a/src/LiveComponent/assets/test/controller/child.test.ts b/src/LiveComponent/assets/test/controller/child.test.ts index 263d6ac2da9..f313c1bb6a8 100644 --- a/src/LiveComponent/assets/test/controller/child.test.ts +++ b/src/LiveComponent/assets/test/controller/child.test.ts @@ -72,6 +72,9 @@ describe('LiveController parent -> child component tests', () => { afterEach(() => { clearDOM(); + if (!fetchMock.done()) { + throw new Error('Mocked requests did not match'); + } fetchMock.reset(); }); diff --git a/src/LiveComponent/assets/test/controller/csrf.test.ts b/src/LiveComponent/assets/test/controller/csrf.test.ts index 39958236fad..17896ce7d60 100644 --- a/src/LiveComponent/assets/test/controller/csrf.test.ts +++ b/src/LiveComponent/assets/test/controller/csrf.test.ts @@ -40,6 +40,9 @@ describe('LiveController CSRF Tests', () => { afterEach(() => { clearDOM(); + if (!fetchMock.done()) { + throw new Error('Mocked requests did not match'); + } fetchMock.reset(); }); @@ -56,7 +59,5 @@ describe('LiveController CSRF Tests', () => { await waitFor(() => expect(element).toHaveTextContent('Comment Saved!')); expect(postMock.lastOptions().headers['X-CSRF-TOKEN']).toEqual('123TOKEN'); - - fetchMock.done(); }); }); diff --git a/src/LiveComponent/assets/test/controller/model.test.ts b/src/LiveComponent/assets/test/controller/model.test.ts index 45e4485b4c3..3bed409f951 100644 --- a/src/LiveComponent/assets/test/controller/model.test.ts +++ b/src/LiveComponent/assets/test/controller/model.test.ts @@ -34,6 +34,9 @@ describe('LiveController data-model Tests', () => { afterEach(() => { clearDOM(); + if (!fetchMock.done()) { + throw new Error('Mocked requests did not match'); + } fetchMock.reset(); }); @@ -52,9 +55,6 @@ describe('LiveController data-model Tests', () => { await waitFor(() => expect(getByLabelText(element, 'Name:')).toHaveValue('Ryan Weaver')); expect(controller.dataValue).toEqual({name: 'Ryan Weaver'}); - // assert all calls were done the correct number of times - fetchMock.done(); - // assert the input is still focused after rendering expect(document.activeElement.dataset.model).toEqual('name'); }); @@ -69,9 +69,6 @@ describe('LiveController data-model Tests', () => { await waitFor(() => expect(getByLabelText(element, 'Name:')).toHaveValue('Jan')); expect(controller.dataValue).toEqual({name: 'Jan'}); - - // assert all calls were done the correct number of times - fetchMock.done(); }); @@ -111,9 +108,6 @@ describe('LiveController data-model Tests', () => { await waitFor(() => expect(getByLabelText(element, 'Name:')).toHaveValue('Ryanguy_')); expect(controller.dataValue).toEqual({name: 'Ryanguy_'}); - // assert all calls were done the correct number of times - fetchMock.done(); - // only 1 render should have ultimately occurred expect(renderCount).toEqual(1); }); @@ -135,9 +129,6 @@ describe('LiveController data-model Tests', () => { await waitFor(() => expect(inputElement).toHaveValue('Ryan Weaver')); expect(controller.dataValue).toEqual({name: 'Ryan Weaver'}); - - // assert all calls were done the correct number of times - fetchMock.done(); }); it('uses data-model when both name and data-model is present', async () => { @@ -157,8 +148,6 @@ describe('LiveController data-model Tests', () => { await waitFor(() => expect(inputElement).toHaveValue('Ryan Weaver')); expect(controller.dataValue).toEqual({name: 'Ryan Weaver'}); - - fetchMock.done(); }); it('uses data-value when both value and data-value is present', async () => { @@ -178,8 +167,6 @@ describe('LiveController data-model Tests', () => { await waitFor(() => expect(inputElement).toHaveValue('first_name')); expect(controller.dataValue).toEqual({name: 'first_name'}); - - fetchMock.done(); }); it('standardizes user[firstName] style models into post.name', async () => { @@ -211,9 +198,6 @@ describe('LiveController data-model Tests', () => { await waitFor(() => expect(inputElement).toHaveValue('Ryan Weaver')); expect(controller.dataValue).toEqual({ user: { firstName: 'Ryan Weaver' } }); - - // assert all calls were done the correct number of times - fetchMock.done(); }); it('updates correctly when live#update is on a parent element', async () => { @@ -245,8 +229,6 @@ describe('LiveController data-model Tests', () => { await waitFor(() => expect(inputElement).toHaveValue('Ryan Weaver')); - fetchMock.done(); - // assert the input is still focused after rendering expect(document.activeElement.getAttribute('name')).toEqual('firstName'); }); @@ -264,9 +246,6 @@ describe('LiveController data-model Tests', () => { await userEvent.type(inputElement, ' WEAVER'); await waitFor(() => expect(inputElement).toHaveValue('Ryan Weaver')); - - // assert all calls were done the correct number of times - fetchMock.done(); }); it('data changed on server should be noticed by controller', async () => { @@ -283,7 +262,5 @@ describe('LiveController data-model Tests', () => { await waitFor(() => expect(inputElement).toHaveValue('Kevin Bond')); expect(controller.dataValue).toEqual({name: 'Kevin Bond'}); - - fetchMock.done(); }); }); diff --git a/src/LiveComponent/assets/test/controller/render.test.ts b/src/LiveComponent/assets/test/controller/render.test.ts index 505842472dd..f29f0ee4b91 100644 --- a/src/LiveComponent/assets/test/controller/render.test.ts +++ b/src/LiveComponent/assets/test/controller/render.test.ts @@ -40,6 +40,9 @@ describe('LiveController rendering Tests', () => { afterEach(() => { clearDOM(); + if (!fetchMock.done()) { + throw new Error('Mocked requests did not match'); + } fetchMock.reset(); }); @@ -57,19 +60,46 @@ describe('LiveController rendering Tests', () => { expect(controller.dataValue).toEqual({name: 'Kevin'}); }); - it('conserves values of fields modified after a render request', async () => { + it('renders over local value in input', async () => { const data = { name: 'Ryan' }; const { element } = await startStimulus(template(data)); fetchMock.get( + // name=Ryan is sent to the server 'http://localhost/_components/my_component?name=Ryan', + // server changes that data template({ name: 'Kevin' }), { delay: 100 } ); - getByText(element, 'Reload').click(); + // type into the input that is not bound to a model userEvent.type(getByLabelText(element, 'Comments:'), '!!'); + getByText(element, 'Reload').click(); await waitFor(() => expect(element).toHaveTextContent('Name: Kevin')); + // value if unmapped input is reset + expect(getByLabelText(element, 'Comments:')).toHaveValue('i like pizza'); + expect(document.activeElement.name).toEqual('comments'); + }); + + it('conserves values of fields modified after a render request IF data-live-ignore', async () => { + const data = { name: 'Ryan' }; + const { element } = await startStimulus(template(data)); + + fetchMock.get( + // name=Ryan is sent to the server + 'http://localhost/_components/my_component?name=Ryan', + // server changes that data + template({ name: 'Kevin' }), + { delay: 100 } + ); + // type into the input that is not bound to a model + const input = getByLabelText(element, 'Comments:'); + input.setAttribute('data-live-ignore', ''); + userEvent.type(input, '!!'); + getByText(element, 'Reload').click(); + + await waitFor(() => expect(element).toHaveTextContent('Name: Kevin')); + // value of unmapped input is NOT reset because of data-live-ignore expect(getByLabelText(element, 'Comments:')).toHaveValue('i like pizza!!'); expect(document.activeElement.name).toEqual('comments'); }); @@ -88,8 +118,10 @@ describe('LiveController rendering Tests', () => { template({ name: 'Kevin' }, true), { delay: 100 } ); + const input = getByLabelText(element, 'Comments:'); + input.setAttribute('data-live-ignore', ''); + userEvent.type(input, '!!'); getByText(element, 'Reload').click(); - userEvent.type(getByLabelText(element, 'Comments:'), '!!'); await waitFor(() => expect(element).toHaveTextContent('Name: Kevin')); expect(getByLabelText(element, 'Comments:')).toHaveValue('i like pizza!!'); diff --git a/src/LiveComponent/assets/test/normalize_attributes_for_comparison.test.ts b/src/LiveComponent/assets/test/normalize_attributes_for_comparison.test.ts new file mode 100644 index 00000000000..f4b83e6a790 --- /dev/null +++ b/src/LiveComponent/assets/test/normalize_attributes_for_comparison.test.ts @@ -0,0 +1,64 @@ +import { normalizeAttributesForComparison } from '../src/normalize_attributes_for_comparison'; + +const createElement = function(html: string): HTMLElement { + const template = document.createElement('template'); + html = html.trim(); + template.innerHTML = html; + + const child = template.content.firstChild; + if (!child || !(child instanceof HTMLElement)) { + throw new Error('Child not found'); + } + + return child; +} + +describe('normalizeAttributesForComparison', () => { + it('makes no changes if value and attribute not set', () => { + const element = createElement('
'); + normalizeAttributesForComparison(element); + expect(element.outerHTML) + .toEqual('
'); + }); + + it('sets the attribute if the value is present', () => { + const element = createElement(''); + element.value = 'set value'; + normalizeAttributesForComparison(element); + expect(element.outerHTML) + .toEqual(''); + }); + + it('sets the attribute to empty if the value is empty', () => { + const element = createElement(''); + element.value = ''; + normalizeAttributesForComparison(element); + expect(element.outerHTML) + .toEqual(''); + }); + + it('changes the value attribute if value is different', () => { + const element = createElement(''); + element.value = 'changed'; + normalizeAttributesForComparison(element); + expect(element.outerHTML) + .toEqual(''); + }); + + it('changes the value attribute on a child', () => { + const element = createElement('
'); + element.querySelector('#child').value = 'changed'; + normalizeAttributesForComparison(element); + expect(element.outerHTML) + .toEqual('
'); + }); + + it('changes the value on multiple levels', () => { + const element = createElement('
'); + element.querySelector('#child').value = 'changed'; + element.querySelector('#grand_child').value = 'changed grand child'; + normalizeAttributesForComparison(element); + expect(element.outerHTML) + .toEqual('
'); + }); +}); diff --git a/src/LiveComponent/src/Resources/doc/index.rst b/src/LiveComponent/src/Resources/doc/index.rst index 9ad6ac1748a..a16b0fe97d1 100644 --- a/src/LiveComponent/src/Resources/doc/index.rst +++ b/src/LiveComponent/src/Resources/doc/index.rst @@ -1477,6 +1477,20 @@ form. But it also makes sure that when the ``textarea`` changes, both the ``value`` model in ``MarkdownTextareaComponent`` *and* the ``post.content`` model in ``EditPostcomponent`` will be updated. +Skipping Updating Certain Elements +---------------------------------- + +Sometimes you may have an element inside a component that you do *not* want to +change whenever your component re-renders. For example, some elements managed by +third-party JavaScript or a form element that is not bound to a model... where you +don't want a re-render to reset the data the user has entered. + +To handle this, add the ``data-live-ignore`` attribute to the element: + +.. code-block:: html + + + Backward Compatibility promise ------------------------------ From 6906cc6508eea4ca15680a4647d0caa69dd60950 Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 1 Feb 2022 06:57:29 +0200 Subject: [PATCH 02/11] Rebuild js dist files --- .../Resources/assets/dist/controller.js | 2 +- .../Resources/assets/dist/controller.js | 5 +- .../assets/dist/live_controller.js | 70 +++++++++++-------- 3 files changed, 46 insertions(+), 31 deletions(-) diff --git a/src/Dropzone/Resources/assets/dist/controller.js b/src/Dropzone/Resources/assets/dist/controller.js index e31134956e4..126e3cd5c7d 100644 --- a/src/Dropzone/Resources/assets/dist/controller.js +++ b/src/Dropzone/Resources/assets/dist/controller.js @@ -43,7 +43,7 @@ class default_1 extends Controller { }); reader.readAsDataURL(file); } - _dispatchEvent(name, payload) { + _dispatchEvent(name, payload = {}) { this.element.dispatchEvent(new CustomEvent(name, { detail: payload })); } } diff --git a/src/LazyImage/Resources/assets/dist/controller.js b/src/LazyImage/Resources/assets/dist/controller.js index 3dda83c1d1d..4cbb82c3d6b 100644 --- a/src/LazyImage/Resources/assets/dist/controller.js +++ b/src/LazyImage/Resources/assets/dist/controller.js @@ -3,11 +3,12 @@ import { Controller } from '@hotwired/stimulus'; class default_1 extends Controller { connect() { const hd = new Image(); + const element = this.element; const srcsetString = this._calculateSrcsetString(); hd.addEventListener('load', () => { - this.element.src = this.srcValue; + element.src = this.srcValue; if (srcsetString) { - this.element.srcset = srcsetString; + element.srcset = srcsetString; } this._dispatchEvent('lazy-image:ready', { image: hd }); }); diff --git a/src/LiveComponent/assets/dist/live_controller.js b/src/LiveComponent/assets/dist/live_controller.js index 22b1c922484..7834400b096 100644 --- a/src/LiveComponent/assets/dist/live_controller.js +++ b/src/LiveComponent/assets/dist/live_controller.js @@ -963,7 +963,7 @@ function setDeepData(data, propertyPath, value) { const finalKey = parts[parts.length - 1]; if (typeof currentLevelData !== 'object') { const lastPart = parts.pop(); - throw new Error(`Cannot set data-model="${propertyPath}". They parent "${parts.join(',')}" data does not appear to be an object (it's "${currentLevelData}"). Did you forget to add exposed={"${lastPart}"} to its LiveProp?`); + throw new Error(`Cannot set data-model="${propertyPath}". The parent "${parts.join('.')}" data does not appear to be an object (it's "${currentLevelData}"). Did you forget to add exposed={"${lastPart}"} to its LiveProp?`); } if (currentLevelData[finalKey] === undefined) { const lastPart = parts.pop(); @@ -1014,6 +1014,26 @@ function haveRenderedValuesChanged(originalDataJson, currentDataJson, newDataJso return keyHasChanged; } +function normalizeAttributesForComparison(element) { + if (element.value) { + element.setAttribute('value', element.value); + } + else if (element.hasAttribute('value')) { + element.setAttribute('value', ''); + } + Array.from(element.children).forEach((child) => { + normalizeAttributesForComparison(child); + }); +} + +function cloneHTMLElement(element) { + const newElement = element.cloneNode(true); + if (!(newElement instanceof HTMLElement)) { + throw new Error('Could not clone element'); + } + return newElement; +} + const DEFAULT_DEBOUNCE = 150; class default_1 extends Controller { constructor() { @@ -1112,23 +1132,12 @@ class default_1 extends Controller { this._makeRequest(null); } _getValueFromElement(element) { - const value = element.dataset.value || element.value; - if (!value) { - const clonedElement = (element.cloneNode()); - if (!(clonedElement instanceof HTMLElement)) { - throw new Error('cloneNode() produced incorrect type'); - } - throw new Error(`The update() method could not be called for "${clonedElement.outerHTML}": the element must either have a "data-value" or "value" attribute set.`); - } - return value; + return element.dataset.value || element.value; } _updateModelFromElement(element, value, shouldRender) { const model = element.dataset.model || element.getAttribute('name'); if (!model) { - const clonedElement = (element.cloneNode()); - if (!(clonedElement instanceof HTMLElement)) { - throw new Error('cloneNode() produced incorrect type'); - } + const clonedElement = cloneHTMLElement(element); throw new Error(`The update() method could not be called for "${clonedElement.outerHTML}": the element must either have a "data-model" or "name" attribute set to the model name.`); } this.$updateModel(model, value, shouldRender, element.hasAttribute('name') ? element.getAttribute('name') : null); @@ -1183,7 +1192,7 @@ class default_1 extends Controller { } const fetchOptions = {}; fetchOptions.headers = { - 'Accept': 'application/vnd.live-component+json', + 'Accept': 'application/vnd.live-component+html', }; if (action) { url += `/${encodeURIComponent(action)}`; @@ -1209,34 +1218,30 @@ class default_1 extends Controller { } const isMostRecent = this.renderPromiseStack.removePromise(thisPromise); if (isMostRecent) { - response.json().then((data) => { - this._processRerender(data); + response.text().then((html) => { + this._processRerender(html, response); }); } }); } - _processRerender(data) { + _processRerender(html, response) { if (this.isWindowUnloaded) { return; } - if (data.redirect_url) { + if (response.headers.get('Location')) { if (typeof Turbo !== 'undefined') { - Turbo.visit(data.redirect_url); + Turbo.visit(response.headers.get('Location')); } else { - window.location.href = data.redirect_url; + window.location.href = response.headers.get('Location') || ''; } return; } - if (!this._dispatchEvent('live:render', data, true, true)) { + if (!this._dispatchEvent('live:render', html, true, true)) { return; } this._onLoadingFinish(); - if (!data.html) { - throw new Error('Missing html key on response JSON'); - } - this._executeMorphdom(data.html); - this.dataValue = data.data; + this._executeMorphdom(html); } _clearWaitingDebouncedRenders() { if (this.renderDebounceTimeout) { @@ -1371,7 +1376,13 @@ class default_1 extends Controller { morphdom(this.element, newElement, { onBeforeElUpdated: (fromEl, toEl) => { if (fromEl.isEqualNode(toEl)) { - return false; + const normalizedFromEl = cloneHTMLElement(fromEl); + normalizeAttributesForComparison(normalizedFromEl); + const normalizedToEl = cloneHTMLElement(toEl); + normalizeAttributesForComparison(normalizedToEl); + if (normalizedFromEl.isEqualNode(normalizedToEl)) { + return false; + } } const controllerName = fromEl.hasAttribute('data-controller') ? fromEl.getAttribute('data-controller') : null; if (controllerName @@ -1380,6 +1391,9 @@ class default_1 extends Controller { && !this._shouldChildLiveElementUpdate(fromEl, toEl)) { return false; } + if (fromEl.hasAttribute('data-live-ignore')) { + return false; + } return true; } }); From 952725cea5a1f52bdf405c5b3a74c4220af32649 Mon Sep 17 00:00:00 2001 From: Kevin Bond Date: Tue, 1 Feb 2022 08:44:43 -0500 Subject: [PATCH 03/11] Ensure JS dist files are current in CI --- .github/workflows/test.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index f7d65303f17..24c4b68ccad 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -22,6 +22,21 @@ jobs: - run: yarn check-lint - run: yarn check-format + js-dist-current: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@master + - run: yarn && yarn build + - name: Check if js dist files are current + id: changes + uses: UnicornGlobal/has-changes-action@v1.0.11 + + - name: Ensure no changes + if: steps.changes.outputs.changed == 1 + run: | + echo "JS dist files need to be rebuilt" + exit 1 + tests-php-low-deps: runs-on: ubuntu-latest steps: From c8b3a283f53b00fe1fbb4342e3d3b46e4ada9e71 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 28 Jan 2022 14:44:34 -0500 Subject: [PATCH 04/11] [Live] Re-extracting form data after submit --- src/LiveComponent/CHANGELOG.md | 5 + src/LiveComponent/composer.json | 1 + .../src/ComponentWithFormTrait.php | 76 ++++--- src/LiveComponent/src/Resources/doc/index.rst | 209 +++++++++++++++--- .../src/Util/LiveFormUtility.php | 60 +++++ .../FormWithCollectionTypeComponent.php | 56 +++++ .../tests/Fixture/Dto/BlogPost.php | 26 +++ .../tests/Fixture/Dto/Comment.php | 19 ++ .../tests/Fixture/Form/BlogPostFormType.php | 46 ++++ .../tests/Fixture/Form/CommentFormType.php | 38 ++++ src/LiveComponent/tests/Fixture/Kernel.php | 2 + .../form_with_collection_type.html.twig | 3 + .../Functional/Form/ComponentWithFormTest.php | 146 ++++++++++++ .../tests/Unit/Util/LiveFormUtilityTest.php | 105 +++++++++ 14 files changed, 729 insertions(+), 63 deletions(-) create mode 100644 src/LiveComponent/src/Util/LiveFormUtility.php create mode 100644 src/LiveComponent/tests/Fixture/Component/FormWithCollectionTypeComponent.php create mode 100644 src/LiveComponent/tests/Fixture/Dto/BlogPost.php create mode 100644 src/LiveComponent/tests/Fixture/Dto/Comment.php create mode 100644 src/LiveComponent/tests/Fixture/Form/BlogPostFormType.php create mode 100644 src/LiveComponent/tests/Fixture/Form/CommentFormType.php create mode 100644 src/LiveComponent/tests/Fixture/templates/components/form_with_collection_type.html.twig create mode 100644 src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php create mode 100644 src/LiveComponent/tests/Unit/Util/LiveFormUtilityTest.php diff --git a/src/LiveComponent/CHANGELOG.md b/src/LiveComponent/CHANGELOG.md index 813134c8000..0b766c29c2c 100644 --- a/src/LiveComponent/CHANGELOG.md +++ b/src/LiveComponent/CHANGELOG.md @@ -5,6 +5,11 @@ - Added `data-live-ignore` attribute. If included in an element, that element will not be updated on re-render. +- `ComponentWithFormTrait` no longer has a `setForm()` method. But there + is also no need to call it anymore. To pass an already-built form to + your component, pass it as a `form` var to `component()`. If you have + a custom `mount()`, you no longer need to call `setForm()` or anything else. + - The Live Component AJAX endpoints now return HTML in all situations instead of JSON. diff --git a/src/LiveComponent/composer.json b/src/LiveComponent/composer.json index 21ff0e06f7e..5e9c66a0942 100644 --- a/src/LiveComponent/composer.json +++ b/src/LiveComponent/composer.json @@ -34,6 +34,7 @@ "doctrine/doctrine-bundle": "^2.0", "doctrine/orm": "^2.7", "symfony/dependency-injection": "^5.4|^6.0", + "symfony/form": "^5.4|^6.0", "symfony/framework-bundle": "^5.4|^6.0", "symfony/phpunit-bridge": "^6.0", "symfony/security-csrf": "^5.4|^6.0", diff --git a/src/LiveComponent/src/ComponentWithFormTrait.php b/src/LiveComponent/src/ComponentWithFormTrait.php index bcd0f20092e..04fbf0e3ac2 100644 --- a/src/LiveComponent/src/ComponentWithFormTrait.php +++ b/src/LiveComponent/src/ComponentWithFormTrait.php @@ -17,6 +17,8 @@ use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException; use Symfony\UX\LiveComponent\Attribute\BeforeReRender; use Symfony\UX\LiveComponent\Attribute\LiveProp; +use Symfony\UX\LiveComponent\Util\LiveFormUtility; +use Symfony\UX\TwigComponent\Attribute\PostMount; /** * @author Ryan Weaver @@ -63,17 +65,29 @@ trait ComponentWithFormTrait */ abstract protected function instantiateForm(): FormInterface; - /** - * Override in your class if you need extra mounted values. - * - * Call $this->setForm($form) manually in that situation - * if you're passing in an initial form. - */ - public function mount(?FormView $form = null) + #[PostMount] + public function postMount(array $data): array { - if ($form) { - $this->setForm($form); + // allow the FormView object to be passed into the component() as "form" + if (\array_key_exists('form', $data)) { + $this->formView = $data['form']; + unset($data['form']); + + if ($this->formView) { + // if a FormView is passed in and it contains any errors, then + // we mark that this entire component has been validated so that + // all validation errors continue showing on re-render + if (LiveFormUtility::doesFormContainAnyErrors($this->formView)) { + $this->isValidated = true; + $this->validatedFields = []; + } + } } + + // set the formValues from the initial form view's data + $this->initializeFormValues(); + + return $data; } /** @@ -87,7 +101,7 @@ public function mount(?FormView $form = null) public function submitFormOnRender(): void { if (!$this->getFormInstance()->isSubmitted()) { - $this->submitForm(false); + $this->submitForm($this->isValidated); } } @@ -103,18 +117,6 @@ public function getForm(): FormView return $this->formView; } - /** - * Call this from mount() if your component receives a FormView. - * - * If your are not passing a FormView into your component, you - * don't need to call this directly: the form will be set for - * you from your instantiateForm() method. - */ - public function setForm(FormView $form): void - { - $this->formView = $form; - } - public function getFormName(): string { if (!$this->formName) { @@ -124,18 +126,19 @@ public function getFormName(): string return $this->formName; } - public function getFormValues(): array + private function initializeFormValues(): void { - if (null === $this->formValues) { - $this->formValues = $this->extractFormValues($this->getForm()); - } - - return $this->formValues; + $this->formValues = $this->extractFormValues($this->getForm()); } private function submitForm(bool $validateAll = true): void { - $this->getFormInstance()->submit($this->formValues); + if (null !== $this->formView) { + throw new \LogicException('The submitForm() method is being called, but the FormView has already been built. Are you calling $this->getForm() - which creates the FormView - before submitting the form?'); + } + + $form = $this->getFormInstance(); + $form->submit($this->formValues); if ($validateAll) { // mark the entire component as validated @@ -146,10 +149,19 @@ private function submitForm(bool $validateAll = true): void // we only want to validate fields in validatedFields // but really, everything is validated at this point, which // means we need to clear validation on non-matching fields - $this->clearErrorsForNonValidatedFields($this->getFormInstance(), $this->getFormName()); + $this->clearErrorsForNonValidatedFields($form, $form->getName()); } - if (!$this->getFormInstance()->isValid()) { + // re-extract the "view" values in case the submitted data + // changed the underlying data or structure of the form + $this->formValues = $this->extractFormValues($this->getForm()); + // remove any validatedFields that do not exist in data anymore + $this->validatedFields = LiveFormUtility::removePathsNotInData( + $this->validatedFields ?? [], + [$form->getName() => $this->formValues], + ); + + if (!$form->isValid()) { throw new UnprocessableEntityHttpException('Form validation failed in component'); } } @@ -192,7 +204,7 @@ private function getFormInstance(): FormInterface return $this->formInstance; } - private function clearErrorsForNonValidatedFields(Form $form, $currentPath = ''): void + private function clearErrorsForNonValidatedFields(Form $form, string $currentPath = ''): void { if (!$currentPath || !\in_array($currentPath, $this->validatedFields, true)) { $form->clearErrors(); diff --git a/src/LiveComponent/src/Resources/doc/index.rst b/src/LiveComponent/src/Resources/doc/index.rst index 89fa291b8fd..76a5ea3ce9b 100644 --- a/src/LiveComponent/src/Resources/doc/index.rst +++ b/src/LiveComponent/src/Resources/doc/index.rst @@ -114,7 +114,7 @@ Oh, and just one more step! Import a routing file from the bundle: That's it! We're ready! -Making your Component “Live” +Making your Component "Live" ---------------------------- If you haven't already, check out the `Twig Component`_ @@ -143,7 +143,7 @@ Suppose you've already built a basic Twig component:: {{ this.randomNumber }} -To transform this into a “live” component (i.e. one that can be +To transform this into a "live" component (i.e. one that can be re-rendered live on the frontend), replace the component's ``AsTwigComponent`` attribute with ``AsLiveComponent`` and add the ``DefaultActionTrait``: @@ -231,14 +231,14 @@ when rendering the component: {{ component('random_number', { min: 5, max: 500 }) }} But what's up with those ``LiveProp`` attributes? A property with the -``LiveProp`` attribute becomes a “stateful” property for this component. -In other words, each time we click the “Generate a new number!” button, +``LiveProp`` attribute becomes a "stateful" property for this component. +In other words, each time we click the "Generate a new number!" button, when the component re-renders, it will *remember* the original values for the ``$min`` and ``$max`` properties and generate a random number between 5 and 500. If you forgot to add ``LiveProp``, when the component re-rendered, those two values would *not* be set on the object. -In short: LiveProps are “stateful properties”: they will always be set +In short: LiveProps are "stateful properties": they will always be set when rendering. Most properties will be LiveProps, with common exceptions being properties that hold services (these don't need to be stateful because they will be autowired each time before the component @@ -282,7 +282,7 @@ as props and these will be added to ``attributes``:
> -data-action=“live#update”: Re-rendering on LiveProp Change +data-action="live#update": Re-rendering on LiveProp Change ---------------------------------------------------------- Could we allow the user to *choose* the ``$min`` and ``$max`` values and @@ -320,7 +320,7 @@ that field! Yes, as you type in a box, the component automatically updates to reflect the new number! Well, actually, we're missing one step. By default, a ``LiveProp`` is -“read only”. For security purposes, a user cannot change the value of a +"read only". For security purposes, a user cannot change the value of a ``LiveProp`` and re-render the component unless you allow it with the ``writable=true`` option: @@ -354,7 +354,7 @@ method has built-in debouncing: it waits for a 150ms pause before sending an Ajax request to re-render. This is built in, so you don't need to think about it. -Lazy Updating on “blur” or “change” of a Field +Lazy Updating on "blur" or "change" of a Field ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Sometimes, you might want a field to re-render only after the user has @@ -375,7 +375,7 @@ happens, add it to the ``data-action`` call: The ``data-action="change->live#update"`` syntax is standard Stimulus syntax, which says: - When the “change” event occurs, call the ``update`` method on the + When the "change" event occurs, call the ``update`` method on the ``live`` controller. .. _deferring-a-re-render-until-later: @@ -397,7 +397,7 @@ clicked). To do that, use the ``updateDefer`` method: + data-action="live#updateDefer" > -Now, as you type, the ``max`` “model” will be updated in JavaScript, but +Now, as you type, the ``max`` "model" will be updated in JavaScript, but it won't, yet, make an Ajax call to re-render the component. Whenever the next re-render *does* happen, the updated ``max`` value will be used. @@ -500,7 +500,7 @@ changes until loading has taken longer than a certain amount of time: Actions ------- -Live components require a single “default action” that is used to +Live components require a single "default action" that is used to re-render it. By default, this is an empty ``__invoke()`` method and can be added with the ``DefaultActionTrait``. Live components are actually Symfony controllers so you can add the normal controller @@ -508,7 +508,7 @@ attributes/annotations (ie ``@Cache``/``@Security``) to either the entire class just a single action. You can also trigger custom actions on your component. Let's pretend we -want to add a “Reset Min/Max” button to our “random number” component +want to add a "Reset Min/Max" button to our "random number" component that, when clicked, sets the min/max numbers back to a default value. First, add a method with a ``LiveAction`` attribute above it that does @@ -549,7 +549,7 @@ will trigger the ``resetMinMax()`` method! After calling that method, the component will re-render like normal, using the new ``$min`` and ``$max`` properties! -You can also add several “modifiers” to the action: +You can also add several "modifiers" to the action: .. code-block:: twig @@ -562,7 +562,7 @@ You can also add several “modifiers” to the action: The ``prevent`` modifier would prevent the form from submitting (``event.preventDefault()``). The ``debounce(300)`` modifier will add -300ms of “debouncing” before the action is executed. In other words, if +300ms of "debouncing" before the action is executed. In other words, if you click really fast 5 times, only one Ajax request will be made! Actions & Services @@ -807,9 +807,9 @@ make it easy to deal with forms:: * The `fieldName` option is needed in this situation because * the form renders fields with names like `name="post[title]"`. * We set `fieldName: ''` so that this live prop doesn't collide - * with that data. The value - initialFormData - could be anything. + * with that data. The value - data - could be anything. */ - #[LiveProp(fieldName: 'initialFormData')] + #[LiveProp(fieldName: 'data')] public ?Post $post = null; /** @@ -881,14 +881,14 @@ This is possible thanks to a few interesting pieces: the user, its validation errors are cleared so that they aren't rendered. -Handling “Cannot dehydrate an unpersisted entity” Errors. +Handling "Cannot dehydrate an unpersisted entity" Errors. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ If you're building a form to create a *new* entity, then when you render the component, you may be passing in a new, non-persisted entity. For example, imagine you create a ``new Post()`` in your controller, -pass this “not-yet-persisted” entity into your template as a ``post`` +pass this "not-yet-persisted" entity into your template as a ``post`` variable and pass *that* into your component: .. code-block:: twig @@ -906,8 +906,8 @@ If you do this, you'll likely see this error: The problem is that the Live component system doesn't know how to transform this object into something that can be sent to the frontend, -called “dehydration”. If an entity has already been saved to the -database, its “id” is sent to the frontend. But if the entity hasn't +called "dehydration". If an entity has already been saved to the +database, its "id" is sent to the frontend. But if the entity hasn't been saved yet, that's not possible. The solution is to pass ``null`` into your component instead of a @@ -933,10 +933,10 @@ behave how you want. If you're re-rendering a field on the ``input`` event (that's the default event on a field, which is fired each time you type in a text -box), then if you type a “space” and pause for a moment, the space will +box), then if you type a "space" and pause for a moment, the space will disappear! -This is because Symfony text fields “trim spaces” automatically. When +This is because Symfony text fields "trim spaces" automatically. When your component re-renders, the space will disappear… as the user is typing! To fix this, either re-render on the ``change`` event (which fires after the text box loses focus) or set the ``trim`` option of your @@ -1070,6 +1070,152 @@ section above) is to add: + data-action="change->live#update" > +Using Actions to Change your Form: CollectionType +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Symfony's `CollectionType`_ can be used to embed a collection of +embedded forms including allowing the user to dynamically add or remove +them. Live components can accomplish make this all possible while +writing zero JavaScript. + +For example, imagine a "Blog Post" form with an embedded "Comment" forms +via the ``CollectionType``:: + + namespace App\Form; + + use Symfony\Component\Form\AbstractType; + use Symfony\Component\Form\Extension\Core\Type\CollectionType; + use Symfony\Component\Form\FormBuilderInterface; + use Symfony\Component\OptionsResolver\OptionsResolver; + use App\Entity\BlogPost; + + class BlogPostFormType extends AbstractType + { + public function buildForm(FormBuilderInterface $builder, array $options) + { + $builder + ->add('title', TextType::class) + // ... + ->add('comments', CollectionType::class, [ + 'entry_type' => CommentFormType::class, + 'allow_add' => true, + 'allow_delete' => true, + 'by_reference' => false, + ]) + ; + } + + public function configureOptions(OptionsResolver $resolver) + { + $resolver->setDefaults(['data_class' => BlogPost::class]); + } + } + +Now, create a Twig component to render the form:: + + namespace App\Twig; + + use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; + use Symfony\Component\Form\FormInterface; + use Symfony\UX\LiveComponent\Attribute\AsLiveComponent; + use Symfony\UX\LiveComponent\Attribute\LiveAction; + use Symfony\UX\LiveComponent\ComponentWithFormTrait; + use Symfony\UX\LiveComponent\DefaultActionTrait; + use App\Entity\BlogPost; + use App\Entity\Comment; + use App\Form\BlogPostFormType; + + #[AsLiveComponent('blog_post_collection_type')] + class BlogPostCollectionTypeComponent extends AbstractController + { + use ComponentWithFormTrait; + use DefaultActionTrait; + + #[LiveProp] + public BlogPost $post; + + protected function instantiateForm(): FormInterface + { + return $this->createForm(BlogPostFormType::class, $this->post); + } + + #[LiveAction] + public function addComment() + { + // "formValues" represents the current data in the form + // this modifies the form to add an extra comment + // the result: another embedded comment form! + // change "comments" to the name of the field that uses CollectionType + $this->formValues['comments'][] = []; + } + + #[LiveAction] + public function removeComment(#[LiveArg] int $index) + { + unset($this->formValues['comments'][$index]); + } + } + +The template for this component has two jobs: (1) render the form +like normal and (2) include links that trigger the ``addComment()`` +and ``removeComment()`` actions: + +.. code-block:: twig + + + {{ form_start(this.form) }} + {{ form_row(this.form.title) }} + +

Comments:

+ {% for key, commentForm in this.form.comments %} + + + {{ form_widget(commentForm) }} + {% endfor %} +
+ + {# avoid an extra label for this field #} + {% do this.form.comments.setRendered %} + + + + + {{ form_end(this.form) }} + + +Done! Behind the scenes, it works like this: + +A) When the user clicks "+ Add Comment", an Ajax request is sent that +triggers the ``addComment()`` action. + +B) ``addComment()`` modifies ``formValues``, which you can think of as +the raw "POST" data of your form. + +C) Still during the Ajax request, the ``formValues`` are "submitted" +into your form. The new key inside of ``$this->formValues['comments']`` +tells the ``CollectionType`` that you want a new, embedded form. + +D) The form is rendered - now with another embedded form! - and the +Ajax call returns with the form (with the new embedded form). + +When the user clicks ``removeComment()``, a similar process happens. + +.. note:: + + When working with Doctrine entities, add ``orphanRemoval: true`` + and ``cascade={"persist"}`` to your ``OneToMany`` relationship. + In this example, these options would be added to the ``OneToMany`` + attribute above the ``Post.comments`` property. These help new + items save and deletes any items whose embedded forms are removed. + Modifying Embedded Properties with the "exposed" Option ------------------------------------------------------- @@ -1091,7 +1237,7 @@ edited:: public Post $post; } -In the template, let's render an HTML form *and* a “preview” area where +In the template, let's render an HTML form *and* a "preview" area where the user can see, as they type, what the post will look like (including rendered the ``content`` through a Markdown filter from the ``twig/markdown-extra`` library): @@ -1187,7 +1333,7 @@ where you want the object on that property to also be validated. Thanks to this setup, the component will now be automatically validated on each render, but in a smart way: a property will only be validated -once its “model” has been updated on the frontend. The system keeps +once its "model" has been updated on the frontend. The system keeps track of which models have been updated (e.g. ``data-action="live#update"``) and only stores the errors for those fields on re-render. @@ -1229,7 +1375,7 @@ method: class="{{ this.getError('post.content') ? 'has-error' : '' }}" >{{ post.content }} -Once a component has been validated, the component will “rememeber” that +Once a component has been validated, the component will "rememeber" that it has been validated. This means that, if you edit a field and the component re-renders, it will be validated again. @@ -1238,7 +1384,7 @@ Real Time Validation As soon as you enable validation, each field will automatically be validated when its model is updated. For example, if you want a single -field to be validated “on change” (when you change the field and then +field to be validated "on change" (when you change the field and then blur the field), update the model via the ``change`` event: .. code-block:: twig @@ -1251,13 +1397,13 @@ blur the field), update the model via the ``change`` event: When the component re-renders, it will signal to the server that this one field should be validated. Like with normal validation, once an -individual field has been validated, the component “remembers” that, and +individual field has been validated, the component "remembers" that, and re-validates it on each render. Polling ------- -You can also use “polling” to continually refresh a component. On the +You can also use "polling" to continually refresh a component. On the **top-level** element for your component, add ``data-poll``: .. code-block:: diff @@ -1279,7 +1425,7 @@ delay for 500ms: data-poll="delay(500)|$render" > -You can also trigger a specific “action” instead of a normal re-render: +You can also trigger a specific "action" instead of a normal re-render: .. code-block:: twig @@ -1313,7 +1459,7 @@ component is its own, isolated universe. But this is not always what you want. For example, suppose you have a parent component that renders a form and a child component that renders -one field in that form. When you click a “Save” button on the parent +one field in that form. When you click a "Save" button on the parent component, that validates the form and re-renders with errors - including a new ``error`` value that it passes into the child: @@ -1376,7 +1522,7 @@ event is dispatched. All components automatically listen to this event. This means that, when the ``markdown_value`` model is updated in the child component, *if* the parent component *also* has a model called ``markdown_value`` it will *also* be updated. This is done as a -“deferred” update +"deferred" update (i.e. :ref:`updateDefer() `). If the model name in your child component (e.g. ``markdown_value``) is @@ -1553,3 +1699,4 @@ bound to Symfony's BC policy for the moment. .. _`dependent form fields`: https://symfony.com/doc/current/form/dynamic_form_modification.html#dynamic-generation-for-submitted-forms .. _`Symfony UX configured in your app`: https://symfony.com/doc/current/frontend/ux.html .. _`Component attributes`: https://symfony.com/bundles/ux-twig-component/current/index.html#component-attributes +.. _`CollectionType`: https://symfony.com/doc/current/form/form_collections.html diff --git a/src/LiveComponent/src/Util/LiveFormUtility.php b/src/LiveComponent/src/Util/LiveFormUtility.php new file mode 100644 index 00000000000..085f090bfc9 --- /dev/null +++ b/src/LiveComponent/src/Util/LiveFormUtility.php @@ -0,0 +1,60 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\UX\LiveComponent\Util; + +use Symfony\Component\Form\FormView; + +final class LiveFormUtility +{ + /** + * Removes the "paths" not present in the $data array. + * + * Given an array of paths - ['name', 'post.title', 'post.description'] - + * and a $data array - ['name' => 'Ryan', 'post' => ['title' => 'Hi there!']] - + * this removes any "paths" not present in the array. + */ + public static function removePathsNotInData(array $paths, array $data): array + { + return array_values(array_filter($paths, static function ($path) use ($data) { + $parts = explode('.', $path); + while (\count($parts) > 0) { + $part = $parts[0]; + if (!\array_key_exists($part, $data)) { + return false; + } + + // reset $parts and go to the next level + unset($parts[0]); + $parts = array_values($parts); + $data = $data[$part]; + } + + // key was found at all levels + return true; + })); + } + + public static function doesFormContainAnyErrors(FormView $formView): bool + { + if (($formView->vars['errors'] ?? null) && \count($formView->vars['errors']) > 0) { + return true; + } + + foreach ($formView->children as $childView) { + if (self::doesFormContainAnyErrors($childView)) { + return true; + } + } + + return false; + } +} diff --git a/src/LiveComponent/tests/Fixture/Component/FormWithCollectionTypeComponent.php b/src/LiveComponent/tests/Fixture/Component/FormWithCollectionTypeComponent.php new file mode 100644 index 00000000000..18757e4a360 --- /dev/null +++ b/src/LiveComponent/tests/Fixture/Component/FormWithCollectionTypeComponent.php @@ -0,0 +1,56 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\UX\LiveComponent\Tests\Fixture\Component; + +use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\Form\FormInterface; +use Symfony\UX\LiveComponent\Attribute\AsLiveComponent; +use Symfony\UX\LiveComponent\Attribute\LiveAction; +use Symfony\UX\LiveComponent\Attribute\LiveArg; +use Symfony\UX\LiveComponent\ComponentWithFormTrait; +use Symfony\UX\LiveComponent\DefaultActionTrait; +use Symfony\UX\LiveComponent\Tests\Fixture\Dto\BlogPost; +use Symfony\UX\LiveComponent\Tests\Fixture\Dto\Comment; +use Symfony\UX\LiveComponent\Tests\Fixture\Form\BlogPostFormType; + +#[AsLiveComponent('form_with_collection_type')] +class FormWithCollectionTypeComponent extends AbstractController +{ + use ComponentWithFormTrait; + use DefaultActionTrait; + + public BlogPost $post; + + public function __construct() + { + $this->post = new BlogPost(); + // start with 1 comment + $this->post->comments[] = new Comment(); + } + + protected function instantiateForm(): FormInterface + { + return $this->createForm(BlogPostFormType::class, $this->post); + } + + #[LiveAction] + public function addComment() + { + $this->formValues['comments'][] = []; + } + + #[LiveAction] + public function removeComment(#[LiveArg] int $index) + { + unset($this->formValues['comments'][$index]); + } +} diff --git a/src/LiveComponent/tests/Fixture/Dto/BlogPost.php b/src/LiveComponent/tests/Fixture/Dto/BlogPost.php new file mode 100644 index 00000000000..be8dea8d272 --- /dev/null +++ b/src/LiveComponent/tests/Fixture/Dto/BlogPost.php @@ -0,0 +1,26 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\UX\LiveComponent\Tests\Fixture\Dto; + +use Symfony\Component\Validator\Constraints\Length; +use Symfony\Component\Validator\Constraints\NotBlank; + +class BlogPost +{ + #[NotBlank(message: 'The title field should not be blank')] + public $title; + + #[Length(min: 100, minMessage: 'The content field is too short')] + public $content; + + public $comments = []; +} diff --git a/src/LiveComponent/tests/Fixture/Dto/Comment.php b/src/LiveComponent/tests/Fixture/Dto/Comment.php new file mode 100644 index 00000000000..72eacac41f1 --- /dev/null +++ b/src/LiveComponent/tests/Fixture/Dto/Comment.php @@ -0,0 +1,19 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\UX\LiveComponent\Tests\Fixture\Dto; + +class Comment +{ + public ?string $content; + + public ?BlogPost $blogPost; +} diff --git a/src/LiveComponent/tests/Fixture/Form/BlogPostFormType.php b/src/LiveComponent/tests/Fixture/Form/BlogPostFormType.php new file mode 100644 index 00000000000..acff3b8eea4 --- /dev/null +++ b/src/LiveComponent/tests/Fixture/Form/BlogPostFormType.php @@ -0,0 +1,46 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Symfony\UX\LiveComponent\Tests\Fixture\Form; + +use Symfony\Component\Form\AbstractType; +use Symfony\Component\Form\Extension\Core\Type\CollectionType; +use Symfony\Component\Form\Extension\Core\Type\TextareaType; +use Symfony\Component\Form\Extension\Core\Type\TextType; +use Symfony\Component\Form\FormBuilderInterface; +use Symfony\Component\OptionsResolver\OptionsResolver; +use Symfony\UX\LiveComponent\Tests\Fixture\Dto\BlogPost; + +class BlogPostFormType extends AbstractType +{ + public function buildForm(FormBuilderInterface $builder, array $options) + { + $builder + ->add('title', TextType::class) + ->add('content', TextareaType::class) + ->add('comments', CollectionType::class, [ + 'entry_type' => CommentFormType::class, + 'allow_add' => true, + 'allow_delete' => true, + ]) + ; + } + + public function configureOptions(OptionsResolver $resolver) + { + $resolver->setDefaults([ + 'csrf_protection' => false, + 'data_class' => BlogPost::class, + ]); + } +} diff --git a/src/LiveComponent/tests/Fixture/Form/CommentFormType.php b/src/LiveComponent/tests/Fixture/Form/CommentFormType.php new file mode 100644 index 00000000000..b801f4de90f --- /dev/null +++ b/src/LiveComponent/tests/Fixture/Form/CommentFormType.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Symfony\UX\LiveComponent\Tests\Fixture\Form; + +use Symfony\Component\Form\AbstractType; +use Symfony\Component\Form\Extension\Core\Type\TextareaType; +use Symfony\Component\Form\FormBuilderInterface; +use Symfony\Component\OptionsResolver\OptionsResolver; +use Symfony\UX\LiveComponent\Tests\Fixture\Dto\Comment; + +class CommentFormType extends AbstractType +{ + public function buildForm(FormBuilderInterface $builder, array $options) + { + $builder + ->add('content', TextareaType::class) + ; + } + + public function configureOptions(OptionsResolver $resolver) + { + $resolver->setDefaults([ + 'csrf_protection' => false, + 'data_class' => Comment::class, + ]); + } +} diff --git a/src/LiveComponent/tests/Fixture/Kernel.php b/src/LiveComponent/tests/Fixture/Kernel.php index 14b5e059efc..16b8eddd016 100644 --- a/src/LiveComponent/tests/Fixture/Kernel.php +++ b/src/LiveComponent/tests/Fixture/Kernel.php @@ -27,6 +27,7 @@ use Symfony\UX\LiveComponent\Tests\Fixture\Component\Component3; use Symfony\UX\LiveComponent\Tests\Fixture\Component\Component6; use Symfony\UX\LiveComponent\Tests\Fixture\Component\ComponentWithAttributes; +use Symfony\UX\LiveComponent\Tests\Fixture\Component\FormWithCollectionTypeComponent; use Symfony\UX\TwigComponent\TwigComponentBundle; use Twig\Environment; @@ -68,6 +69,7 @@ protected function configureContainer(ContainerBuilder $c, LoaderInterface $load $c->register(Component3::class)->setAutoconfigured(true)->setAutowired(true); $c->register(Component6::class)->setAutoconfigured(true)->setAutowired(true); $c->register(ComponentWithAttributes::class)->setAutoconfigured(true)->setAutowired(true); + $c->register(FormWithCollectionTypeComponent::class)->setAutoconfigured(true)->setAutowired(true); $c->loadFromExtension('framework', [ 'secret' => 'S3CRET', diff --git a/src/LiveComponent/tests/Fixture/templates/components/form_with_collection_type.html.twig b/src/LiveComponent/tests/Fixture/templates/components/form_with_collection_type.html.twig new file mode 100644 index 00000000000..56334201bc5 --- /dev/null +++ b/src/LiveComponent/tests/Fixture/templates/components/form_with_collection_type.html.twig @@ -0,0 +1,3 @@ + + {{ form(this.form) }} + diff --git a/src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php b/src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php new file mode 100644 index 00000000000..d64de1326aa --- /dev/null +++ b/src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php @@ -0,0 +1,146 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Symfony\UX\LiveComponent\Tests\Functional\EventListener; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Symfony\Component\DomCrawler\Crawler; +use Symfony\Component\Form\FormFactoryInterface; +use Symfony\UX\LiveComponent\LiveComponentHydrator; +use Symfony\UX\LiveComponent\Tests\Fixture\Component\FormWithCollectionTypeComponent; +use Symfony\UX\LiveComponent\Tests\Fixture\Form\BlogPostFormType; +use Symfony\UX\TwigComponent\ComponentFactory; +use Zenstruck\Browser\Response\HtmlResponse; +use Zenstruck\Browser\Test\HasBrowser; +use Zenstruck\Foundry\Test\Factories; +use Zenstruck\Foundry\Test\ResetDatabase; + +/** + * @author Jakub Caban + */ +class ComponentWithFormTest extends KernelTestCase +{ + use Factories; + use HasBrowser; + use ResetDatabase; + + public function testFormValuesRebuildAfterFormChanges(): void + { + /** @var LiveComponentHydrator $hydrator */ + $hydrator = self::getContainer()->get('ux.live_component.component_hydrator'); + /** @var ComponentFactory $factory */ + $factory = self::getContainer()->get('ux.twig_component.component_factory'); + $component = $factory->create('form_with_collection_type'); + + $dehydrated = $hydrator->dehydrate($component); + $token = null; + + $this->browser() + ->get('/_components/form_with_collection_type?'.http_build_query($dehydrated)) + ->use(function (HtmlResponse $response) use (&$dehydrated, &$token) { + // mimic user typing + $dehydrated['blog_post_form']['content'] = 'changed description by user'; + $dehydrated['validatedFields'] = ['blog_post_form.content']; + $token = $response->crawler()->filter('div')->first()->attr('data-live-csrf-value'); + }) + + // post to action, which will add a new embedded comment + ->post('/_components/form_with_collection_type/addComment', [ + 'body' => $dehydrated, + 'headers' => ['X-CSRF-TOKEN' => $token], + ]) + ->assertStatus(422) + // look for original embedded form + ->assertContains('') + // check that validation happened and stuck + ->assertContains('The content field is too short') + // make sure the title field did not suddenly become validated + ->assertNotContains('The title field should not be blank') + ->use(function (Crawler $crawler) use (&$dehydrated, &$token) { + $div = $crawler->filter('[data-controller="live"]'); + $liveData = json_decode($div->attr('data-live-data-value'), true); + // make sure the 2nd collection type was initialized, that it didn't + // just "keep" the empty array that we set it to in the component + $this->assertEquals( + [ + ['content' => ''], + ['content' => ''], + ], + $liveData['blog_post_form']['comments'] + ); + + // grab the latest live data + $dehydrated = $liveData; + // fake that this field was being validated + $dehydrated['validatedFields'][] = 'blog_post_form.0.comments.content'; + $token = $div->attr('data-live-csrf-value'); + }) + + // post to action, which will remove the original embedded comment + ->post('/_components/form_with_collection_type/removeComment?'.http_build_query(['args' => 'index=0']), [ + 'body' => $dehydrated, + 'headers' => ['X-CSRF-TOKEN' => $token], + ]) + ->assertStatus(422) + // the original embedded form should be gone + ->assertNotContains('