Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add Collection component
  • Loading branch information
dunglas committed Jul 18, 2022
commit e901026aa1d2e2cd2b27a5796962bc8fc4b5dcca
8 changes: 8 additions & 0 deletions src/Collection/.gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/.gitattributes export-ignore
/.gitignore export-ignore
/.symfony.bundle.yaml export-ignore
/phpunit.xml.dist export-ignore
/phpstan.neon.dist export-ignore
/Resources/assets/test export-ignore
/Resources/assets/jest.config.js export-ignore
/Tests export-ignore
12 changes: 12 additions & 0 deletions src/Collection/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/.php_cs.cache
/.php_cs
/.phpunit.result.cache
/composer.phar
/composer.lock
/phpunit.xml
/vendor/
/Tests/app/var
/Tests/app/public/build/
node_modules/
package-lock.json
yarn.lock
3 changes: 3 additions & 0 deletions src/Collection/.symfony.bundle.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
branches: ["2.x"]
maintained_branches: ["2.x"]
doc_dir: "Resources/doc"
16 changes: 16 additions & 0 deletions src/Collection/CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Contributing

Install the test app:

$ composer install
$ cd Tests/app
$ yarn install
$ yarn build

Start the test app:

$ symfony serve

## Run tests

$ php vendor/bin/simple-phpunit
21 changes: 21 additions & 0 deletions src/Collection/CollectionBundle.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\UX\Collection;

use Symfony\Component\HttpKernel\Bundle\Bundle;

/**
* @author Kévin Dunglas <[email protected]>
*/
final class CollectionBundle extends Bundle
{
}
19 changes: 19 additions & 0 deletions src/Collection/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
Copyright (c) 2021 Kévin Dunglas
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2022? 😌


Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is furnished
to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
11 changes: 11 additions & 0 deletions src/Collection/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Symfony UX Collection

**This repository is a READ-ONLY sub-tree split**. See
https://github.com/symfony/ux to create issues or submit pull requests.

## Resources

- [Documentation](https://symfony.com/bundles/ux-collection/current/index.html)
- [Report issues](https://github.com/symfony/ux/issues) and
[send Pull Requests](https://github.com/symfony/ux/pulls)
in the [main Symfony UX repository](https://github.com/symfony/ux)
71 changes: 71 additions & 0 deletions src/Collection/Resources/assets/dist/controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { Controller } from '@hotwired/stimulus';

const DEFAULT_ITEMS_SELECTOR = ':scope > :is(div, fieldset)';
var ButtonType;
(function (ButtonType) {
ButtonType["Add"] = "add";
ButtonType["Delete"] = "delete";
})(ButtonType || (ButtonType = {}));
class controller extends Controller {
connect() {
this.connectCollection(this.element);
}
connectCollection(parent) {
parent.querySelectorAll('[data-prototype]').forEach((el) => {
const collectionEl = el;
const items = this.getItems(collectionEl);
collectionEl.dataset.currentIndex = items.length.toString();
this.addAddButton(collectionEl);
this.getItems(collectionEl).forEach(itemEl => this.addDeleteButton(collectionEl, itemEl));
});
}
getItems(collectionElement) {
return collectionElement.querySelectorAll(collectionElement.dataset.itemsSelector || DEFAULT_ITEMS_SELECTOR);
}
createButton(collectionEl, buttonType) {
const buttonTemplateID = collectionEl.dataset[`${buttonType}ButtonTemplateId`];
if (buttonTemplateID && 'content' in document.createElement('template')) {
const buttonTemplate = document.getElementById(buttonTemplateID);
if (!buttonTemplate)
throw new Error(`element with ID "${buttonTemplateID}" not found`);
return buttonTemplate.content.cloneNode(true);
}
const button = document.createElement('button');
button.type = 'button';
button.textContent = buttonType === ButtonType.Add ? 'Add' : 'Delete';
return button;
}
addItem(collectionEl) {
const currentIndex = collectionEl.dataset.currentIndex;
collectionEl.dataset.currentIndex++;
const collectionNamePattern = collectionEl.id.replace(/_/g, '(?:_|\\[|]\\[)');
const prototype = collectionEl.dataset.prototype
.replace('__name__label__', currentIndex)
.replace(new RegExp(`(${collectionNamePattern}(?:_|]\\[))__name__`, 'g'), `$1${currentIndex}`);
const fakeEl = document.createElement('div');
fakeEl.innerHTML = prototype;
const itemEl = fakeEl.firstElementChild;
this.connectCollection(itemEl);
this.addDeleteButton(collectionEl, itemEl);
const items = this.getItems(collectionEl);
items.length ? items[items.length - 1].insertAdjacentElement('afterend', itemEl) : collectionEl.prepend(itemEl);
}
addAddButton(collectionEl) {
const addButton = this.createButton(collectionEl, ButtonType.Add);
addButton.onclick = (e) => {
e.preventDefault();
this.addItem(collectionEl);
};
collectionEl.appendChild(addButton);
}
addDeleteButton(collectionEl, itemEl) {
const deleteButton = this.createButton(collectionEl, ButtonType.Delete);
deleteButton.onclick = (e) => {
e.preventDefault();
itemEl.remove();
};
itemEl.appendChild(deleteButton);
}
}

export { controller as default };
1 change: 1 addition & 0 deletions src/Collection/Resources/assets/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('../../../../jest.config.js');
24 changes: 24 additions & 0 deletions src/Collection/Resources/assets/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"name": "@symfony/ux-collection",
"description": "Support for collection embedding with Symfony Form",
"license": "MIT",
"main": "dist/controller.js",
"module": "dist/controller.js",
"version": "1.0.0",
"symfony": {
"controllers": {
"collection": {
"main": "dist/controller.js",
"webpackMode": "eager",
"fetch": "eager",
"enabled": true
}
}
},
"peerDependencies": {
"@hotwired/stimulus": "^3.0.0"
},
"devDependencies": {
"@hotwired/stimulus": "^3.0.0"
}
}
98 changes: 98 additions & 0 deletions src/Collection/Resources/assets/src/controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { Controller } from '@hotwired/stimulus';

const DEFAULT_ITEMS_SELECTOR = ':scope > :is(div, fieldset)';

interface CollectionDataset extends DOMStringMap {
prototype: string;
currentIndex: string;
itemsSelector?: string;
addButtonTemplateId?: string;
disableAddButton?: string;
deleteButtonTemplateId?: string;
disableDeleteButton?: string;
}

enum ButtonType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a const enum instead, to inline the add and remove string in the emitted JS code instead of emitting an enum object.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the code and using const enum isn't possible anymore (IIRC, const enum causes various issues anyway).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another alternative is to use a tagged union 'add' | 'remove', which will then produce more efficient JS code.

Add = 'add',
Delete = 'delete',
}

export default class extends Controller {
connect() {
this.connectCollection(this.element as HTMLElement);
}

connectCollection(parent: HTMLElement) {
parent.querySelectorAll('[data-prototype]').forEach((el) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still want to add here a comment that this the most questionable line in my point of view.

In #400 I implemented the JS the way I think it should be implemented that every collection itself has its collection_controller and not that one controller controls all collections.

So we even don't need to wrap the whole form with a data-controller.

Why I would avoid the data-prototype selector here is that for example I have inside my current forms other Form Types using [data-prototye] which are even inside collection types, this component currently would identify them also as "collection" which they are not they just use the data-prototype for different usecases.

Via a form extension like in #400 we can add a better way by adding data-controller to all collection which would solve this problem and even allows to create collection type with a custom stimulus controller like in #90 or #397 also allows extending the current implementaton via events.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make this selector configurable and split this controller in two (the main one in case you want all collections to be automatically handled, and a nested one). WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still is hard to find here a correct selector as the main problems is that the component is registered on the form tag and handles multiple instances of collections.

As I did use component libraries over the last years which work very simular to stimulus I would really not recommend creating components like this which handles multiple instances. Instead I would make sure to have a collection_controller which handles the a single collection, which should make also the code simpler.

What problem do you see that the JS component handles just a single collection and adding a data-controller via form extension to the collection row like in #400.

const collectionEl = el as HTMLElement;
const items = this.getItems(collectionEl);
collectionEl.dataset.currentIndex = items.length.toString();

this.addAddButton(collectionEl);

this.getItems(collectionEl).forEach((itemEl) => this.addDeleteButton(collectionEl, itemEl as HTMLElement));
});
}

getItems(collectionElement: HTMLElement) {
return collectionElement.querySelectorAll(collectionElement.dataset.itemsSelector || DEFAULT_ITEMS_SELECTOR);
}

createButton(collectionEl: HTMLElement, buttonType: ButtonType): HTMLElement {
const buttonTemplateID = collectionEl.dataset[`${buttonType}ButtonTemplateId`];
if (buttonTemplateID && 'content' in document.createElement('template')) {
// Get from template
const buttonTemplate = document.getElementById(buttonTemplateID) as HTMLTemplateElement | null;
if (!buttonTemplate) throw new Error(`element with ID "${buttonTemplateID}" not found`);

return buttonTemplate.content.cloneNode(true) as HTMLElement;
}

// If no template is provided, create a raw HTML button
const button = document.createElement('button') as HTMLButtonElement;
button.type = 'button';
button.textContent = buttonType === ButtonType.Add ? 'Add' : 'Delete';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should support a way to customize the labels. The common use case is to translate them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is supported using custom templates. Isn't it enough?


return button;
}

addItem(collectionEl: HTMLElement) {
const currentIndex = (collectionEl.dataset as CollectionDataset).currentIndex;
(collectionEl.dataset.currentIndex as unknown as number)++;

const collectionNamePattern = collectionEl.id.replace(/_/g, '(?:_|\\[|]\\[)');

const prototype = (collectionEl.dataset.prototype as string) // We're sure that dataset.prototype exists, because of the CSS selector used in connect()
.replace('__name__label__', currentIndex)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The __name__ part is configuration in CollectionType (which is necessary to support nested collection types, as you should then use different prototype names for them))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually my code supports nested collections even when not using another placeholder, but I'll add support for this option!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it does not. The prototype of the outer collection actually contains the attribute storing the prototype of the inner collection (as it contains the whole inner collection). If you use the same placeholder for both, the replacement done here will replace both placeholders inside the inner prototype, breaking the inner collection.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try doing a nested collection where you add multiple items in the inner collection of an item added in the outer collection, and then look at the submitted data in the Request (you need to inspect the request itself, not the data decoded in $_POST as duplicate keys would be lost during the conversion to a PHP array)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the replacement done here will replace both placeholders inside the inner prototype

I didn't handle the label yet, but for __name__, the regex only replaces the first occurrence, which should be enough to fix this issue. I'll double-check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replacing only the first occurrence is also broken. __name__ will appear multiple time in the prototype when the entry type is a compound type (as the name of each child form will have the placeholder in its name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first occurence in every string: https://github.com/symfony/ux/pull/398/files#diff-0c84303846c77e8d3377b23960aed4a1dfc3db0bb6206c7435ef6fe890a7fa1bR49

Unless I'm missing something, this works with the current example I provided for test purpose.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dunglas AFAICT, this would replace both placecholders in name="root[outer_collection][__name__][inner_collection][__name][field_1]"
And trying to do clever tricks based on the quote delimiting attributes (to replace the first occurrence in each attribute) is a bad idea: in the prototype attribute of the nested collection, those would be HTML entities and not actual quotes.

Note that you need to add multiple items in each collection to notice the issue (if you only add one outer item with a single inner item, you won't notice whether the [0] come from the replacements done by the outer or inner item)

.replace(new RegExp(`(${collectionNamePattern}(?:_|]\\[))__name__`, 'g'), `$1${currentIndex}`);

const fakeEl = document.createElement('div');
fakeEl.innerHTML = prototype;
const itemEl = fakeEl.firstElementChild as HTMLElement;

this.connectCollection(itemEl);

this.addDeleteButton(collectionEl, itemEl);

const items = this.getItems(collectionEl);
items.length ? items[items.length - 1].insertAdjacentElement('afterend', itemEl) : collectionEl.prepend(itemEl);
}

addAddButton(collectionEl: HTMLElement) {
const addButton = this.createButton(collectionEl, ButtonType.Add);
addButton.onclick = (e) => {
e.preventDefault();
this.addItem(collectionEl);
};
collectionEl.appendChild(addButton);
}

addDeleteButton(collectionEl: HTMLElement, itemEl: HTMLElement) {
const deleteButton = this.createButton(collectionEl, ButtonType.Delete);
deleteButton.onclick = (e) => {
e.preventDefault();
itemEl.remove();
};
itemEl.appendChild(deleteButton);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #90 where was discussed that it would be nice that the buttons are rendered by the symfony form theme. That make it special for design systems easier so the can directly use there already designed button_widget and bootstrap and other themes already ships with a already good looking button. currently buttons are here if not explicit a template is defined rendered via this js component. in #400 I used the twig extension to add the buttons already to the twig and to the prototype, which did make also the JS a lot smaller which I think is great from maintainance point of view.

Copy link
Member Author

@dunglas dunglas Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My long-term goal is to totally remove data-prototype from core, generate <template> elements instead (which are better suited for this use case), and provide default buttons directly in the core form themes (as <template> elements too). However, during the deprecation process, we'll need to support both this new approach and data-prototype (for the "plug & play" DX) until Symfony 7.

I'll open the PR on core in the next few days.

Copy link
Contributor

@alexander-schranz alexander-schranz Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree still that template are better suited for the data-prototype. Still I recommed to have the buttons rendered already in twig because adding them via JS will trigger else a Layout shift and google and lighthouse hate layout shifts.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that's the plan (and will be the default in Symfony 7!)

}
40 changes: 40 additions & 0 deletions src/Collection/Resources/assets/test/controller.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

'use strict';

import { Application } from '@hotwired/stimulus';
import { getByTestId } from '@testing-library/dom';
import { clearDOM, mountDOM } from '@symfony/stimulus-testing';
import CollectionController from '../src/controller';

const startStimulus = () => {
const application = Application.start();
application.register('symfony--ux-collection--collection', CollectionController);
};

/* eslint-disable no-undef */
describe('CollectionController', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any way to make the tests cover the interactions (add and delete) ? I'm fearing that they might be broken right now due to adding the listeners on the DocumentFragment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I planned to add a Panther test.

Copy link
Contributor

@alexander-schranz alexander-schranz Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need panther here if we use the testing library. I currently could not get your branch running. But something into this direction should work to test add and remove:

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <[email protected]>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

'use strict';

import { Application } from '@hotwired/stimulus';
import {fireEvent, getByTestId, getByText, waitFor} from '@testing-library/dom';
import { clearDOM, mountDOM } from '@symfony/stimulus-testing';
import CollectionController from '../src/controller';

const startStimulus = () => {
    const application = Application.start();
    application.register('symfony--ux-collection--collection', CollectionController);
};

const emptyCollection = '<div data-testid="collection" data-controller="symfony--ux-collection--collection"></div>';

const simpleCollection = '' +
    '    <template id="addButton">' +
    '        <button type="button" class="add-button">Add button</button>' +
    '    </template>' +
    '' +
    '    <template id="deleteButton">' +
    '        <button type="button" class="delete-button">Delete button</button>' +
    '    </template>' +
    '' +
    '<form name="game" method="post" data-controller="symfony--ux-collection--collection" data-add-button="addButton" data-delete-button="deleteButton">' +
    '   <div id="game">' +
    '       <div>' +
    '           <label class="required">Teams</label>' +
    '           <div id="game_teams" data-prototype="&lt;div class=&quot;team-entry&quot;&gt;&lt;label class=&quot;required&quot;&gt;__name__label__&lt;/label&gt;&lt;div id=&quot;game_teams___name__&quot;&gt;&lt;div&gt;&lt;label for=&quot;game_teams___name___name&quot; class=&quot;required&quot;&gt;Name&lt;/label&gt;&lt;input type=&quot;text&quot; id=&quot;game_teams___name___name&quot; name=&quot;game[teams][__name__][name]&quot; required=&quot;required&quot; data-controller=&quot;test&quot; /&gt;&lt;/div&gt;&lt;/div&gt;&lt;/div&gt;">' +
    '           </div>' +
    '       </div>' +
    '   <div>' +
    '   <button type="submit" id="game_submit" name="game[submit]">Submit</button>' +
    '   </div>' +
    '</form>'

const nestedCollection = '' +
    '    <template id="addButton">' +
    '        <button type="button" class="add-button">Add button</button>' +
    '    </template>' +
    '' +
    '    <template id="deleteButton">' +
    '        <button type="button" class="delete-button">Delete button</button>' +
    '    </template>' +
    '' +
    '<form name="game" method="post" data-controller="symfony--ux-collection--collection" data-add-button="addButton" data-delete-button="deleteButton">' +
    '   <div id="game">' +
    '       <div>' +
    '           <label class="required">Teams</label>' +
    '           <div id="game_teams" data-prototype="&lt;div&gt;&lt;label class=&quot;required&quot;&gt;__name__label__&lt;/label&gt;&lt;div id=&quot;game_teams___name__&quot;&gt;&lt;div&gt;&lt;label for=&quot;game_teams___name___name&quot; class=&quot;required&quot;&gt;Name&lt;/label&gt;&lt;input type=&quot;text&quot; id=&quot;game_teams___name___name&quot; name=&quot;game[teams][__name__][name]&quot; required=&quot;required&quot; data-controller=&quot;test&quot; /&gt;&lt;/div&gt;&lt;div&gt;&lt;label class=&quot;required&quot;&gt;Players&lt;/label&gt;&lt;div id=&quot;game_teams___name___players&quot; data-prototype=&quot;&amp;lt;div&amp;gt;&amp;lt;label class=&amp;quot;required&amp;quot;&amp;gt;__name__label__&amp;lt;/label&amp;gt;&amp;lt;div id=&amp;quot;game_teams___name___players___name__&amp;quot;&amp;gt;&amp;lt;div&amp;gt;&amp;lt;label for=&amp;quot;game_teams___name___players___name___firstName&amp;quot; class=&amp;quot;required&amp;quot;&amp;gt;First name&amp;lt;/label&amp;gt;&amp;lt;input type=&amp;quot;text&amp;quot; id=&amp;quot;game_teams___name___players___name___firstName&amp;quot; name=&amp;quot;game[teams][__name__][players][__name__][firstName]&amp;quot; required=&amp;quot;required&amp;quot; /&amp;gt;&amp;lt;/div&amp;gt;&amp;lt;div&amp;gt;&amp;lt;label for=&amp;quot;game_teams___name___players___name___lastName&amp;quot; class=&amp;quot;required&amp;quot;&amp;gt;Last name&amp;lt;/label&amp;gt;&amp;lt;input type=&amp;quot;text&amp;quot; id=&amp;quot;game_teams___name___players___name___lastName&amp;quot; name=&amp;quot;game[teams][__name__][players][__name__][lastName]&amp;quot; required=&amp;quot;required&amp;quot; /&amp;gt;&amp;lt;/div&amp;gt;&amp;lt;/div&amp;gt;&amp;lt;/div&amp;gt;&quot;&gt;&lt;/div&gt;&lt;/div&gt;&lt;/div&gt;&lt;/div&gt;">' +
    '           </div>' +
    '       </div>' +
    '   <div>' +
    '   <button type="submit" id="game_submit" name="game[submit]">Submit</button>' +
    '   </div>' +
    '</form>'

/* eslint-disable no-undef */
describe('CollectionController', () => {
    startStimulus();

    afterEach(() => {
        clearDOM();
    });

    it('connects', async () => {
        const container = mountDOM(emptyCollection);

        // smoke test
        expect(getByTestId(container, 'collection')).toHaveAttribute('data-controller', 'symfony--ux-collection--collection');
    });

    it('add a new collection', async () => {
        const container = mountDOM(simpleCollection);

        await waitFor(() => getByText(container, 'Add button'));

        fireEvent(
            getByText(container, 'Add button'),
            new MouseEvent('click', {
                bubbles: true,
                cancelable: true,
            }),
        )

        expect(container.querySelectorAll('.team-entry').length).toBe(1);
    });

    it('remove a collection', async () => {
        const container = mountDOM(simpleCollection);

        await waitFor(() => getByText(container, 'Add button'));

        fireEvent(
            getByText(container, 'Add button'),
            new MouseEvent('click', {
                bubbles: true,
                cancelable: true,
            }),
        )

        expect(container.querySelectorAll('.team-entry').length).toBe(1);

        await waitFor(() => getByText(container, 'Delete button'));

        fireEvent(
            getByText(container, 'Delete button'),
            new MouseEvent('click', {
                bubbles: true,
                cancelable: true,
            }),
        )

        expect(container.querySelectorAll('.team-entry').length).toBe(0);
    });
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dunglas could get it running and updated the code snippet. Currently only test for simple add and remove and no tests yet for the nested collection but already created the required html for it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it's better to create an E2E test here because we want to be sure that the HTML generated by the form component is still compatible with the JS code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Panther tests would still be worthy as they would ensure that the submitted data is the one expected by the backend code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense specially @stof usecase to make sure that the submitted data is the one we are expecting. Still I think jest tests should also be added. I moved it to a PR:

dunglas#5

let container: any;

beforeEach(() => {
container = mountDOM('<div data-testid="collection" data-controller="symfony--ux-collection--collection"></div>');
});

afterEach(() => {
clearDOM();
});

it('connects', async () => {
startStimulus();

// smoke test
expect(getByTestId(container, 'collection')).toHaveAttribute('data-controller', 'symfony--ux-collection--collection');
});
});
40 changes: 40 additions & 0 deletions src/Collection/Tests/app/Entity/Game.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

declare(strict_types=1);

namespace App\Entity;

/**
* @author Kévin Dunglas <[email protected]>
*/
class Game
{
public \DateTimeImmutable $date;
/**
* @var Team[]
*/
private array $teams = [];

public function addTeam(Team $team): void
{
$this->teams[] = $team;
$this->teams = array_values($this->teams);

dump($this->teams);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably be removed

}

public function removeTeam(Team $team): void
{
if (false === $i = array_search($team, $this->teams, true)) {
return;
}

unset($this->teams[$i]);
$this->teams = array_values($this->teams);
}

public function getTeams(): array
{
return $this->teams;
}
}
14 changes: 14 additions & 0 deletions src/Collection/Tests/app/Entity/Player.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace App\Entity;

/**
* @author Kévin Dunglas <[email protected]>
*/
class Player
{
public string $firstName;
public string $lastName;
}
Loading