UX FormCollection #90
Update src/FormCollection/README.md
Co-authored-by: jmsche <[email protected]>
- Loading branch information
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the biggest question make for me with this very nice PR. If we provide a built-in form theme to "rendering everything for them", then we will also need a Bootstrap 4/5 theme... and a tailwind theme. And, if the user needs to customize how things look, then they need to override a fairly complex form theme that we've created here.
I'm not definitely against this. However, I'd like to see if we can document how this new feature could be used if there were no form themes included. For example,
{% macro commentFormRow(commentForm) %} <div class="col-4" data-symfony--ux-form-collection--collection-target="entry" > {{ form_errors(commentForm) }} {{ form_row(commentForm.content) }} {{ form_row(commentForm.otherField) }} <a data-action="symfony--ux-form-collection--collection#delete"> Remove </a> </div> {% endblock %} <div class="row" {{ stimulus_controller('symfony/ux-form-collection/collection', { prototype: _self.commentFormRow(form.comments.vars.prototype), }) }} > {% for commentForm in form.comments %} {{ _self.commentFormRow(commentForm) }} {% endfor %} <a data-action="symfony--ux-form-collection--collection#add"> Add Another </a> </div>This would give me full control over everything, including the buttons. The Stimulus controller would just handle making it all work :). The worst pieces are the super-ugly
data-actionanddata-...-targetattributes. But this would look much nicer if we merged symfony/webpack-encore-bundle#124There may be some other missing pieces... but in theory, this should be enough to work. The "delete" button should not need a
data-index-entryattribute, as we can look to its ancestors for the "closest" entry target to know which item is being removed.So, this is my thinking :). No form theme, but we make it as dead-simple as possible for users to render things themselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My idea was to make a component very easy to use.
It's true that template overloading can be complicated due to attributes related to Stimulus.
With these 2 templates (
@FormCollection/form_theme_div.html.twig,@FormCollection/form_theme_table.html.twig) we cover a maximum of use cases.For your proposal, there should be explicit errors when the controller is wrong or the action is not correctly entered.
I think you need to know Stimulus to be able to set up HTML according to the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make a better judgement, I'll need some time to play with this in a real project :). Would you be willing/able to push a demo app to GitHub that uses this (with some quick setup instructions)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you have my repo for my test :
https://github.com/stakovicz/ux-collection-test/
Enjoy !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made an update.
Now you can use a predefined theme or not.
You have the choice.
I need help to do more tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @weaverryan,
Any reason to use a macro instead of a proper form theme here?
The macro forces you to use it everywhere it's needed (eg. prototype & entries as in your example) while a "real" form theme would apply automatically to both.