Hard-coded native attributes #3
Open
Conversation
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
…em/helper (kbond) This PR was merged into the 2.x branch. Discussion ---------- [WIP][TwigComponent] add component attribute system/helper | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Tickets | n/a | License | MIT A common pattern with other component library's (ie blade components/vue) is to pass html attributes to your component that are set on the component's root node. This proposal adds a way to do this with twig components. Enable on your component by adding the `HasAttributesTrait`. Attributes are any data passed to `component()` that cannot be mounted on the component itself. This extra data is added to a `ComponentAttributes` object that lives as a public property on your component (available as `attributes` in your component's template). This should all work out-of-the box with live components. Todo: - [x] Tests - [x] Documentation - [x] symfony#240 - [x] symfony#241 - [x] Merge `init_live_component()` into attributes when available - [x] Document `PreRender` event. - [x] Adjust `ComponentAttributes` api (defaults instead of merge) - [ ] `attributes` always available in normal twig component templates (not sure about this - it would effectively make attributes _native_ but it a lame way) - [x] replace `init_live_component()` with `attributes` in docs - [x] Finalize docs (versionadded tags) ## Usage To use, add the `HasAttributesTrait` to your component: ```php #[AsTwigComponent('my_component')] class MyComponent { use HasAttributesTrait; } ``` Then, in your component's template: ```twig {# templates/components/my_component.html.twig #} <div{{ attributes }}> My Component! </div> ``` When rendering the component, you can pass an array of html attributes to add: ```twig {{ component('my_component', { class: 'foo', style: 'color:red' }) }} {# renders as: #} <div class="foo" style="color:red"> My Component! </div> ``` ### Defaults & Merging In your component template, you can set defaults that are merged with passed attributes. The passed attributes override the default with the exception of *class*. For class, the defaults are prepended: ```twig {# templates/components/my_component.html.twig #} <button{{ attributes.defaults({ class: 'bar', type: 'button' }) }}> My Component! </button> {# render component #} {{ component('my_component', { style: 'color:red' }) }} {{ component('my_component', { class: 'foo', type: 'submit' }) }} {# renders as: #} <div class="bar" style="color:red"> My Component! </div> <div class="bar foo" type="submit"> My Component! </div> ``` ### Only ```twig {# templates/components/my_component.html.twig #} <div{{ attributes.only('class')) }}> My Component! </div> {# render component #} {{ component('my_component', { class: 'foo', style: 'color:red' }) }} {# renders as: #} <div class="foo"> My Component! </div> ``` ### Without ```twig {# templates/components/my_component.html.twig #} <div{{ attributes.without('class')) }}> My Component! </div> {# render component #} {{ component('my_component', { class: 'foo', style: 'color:red' }) }} {# renders as: #} <div style="color:red"> My Component! </div> ``` ### Live Components We removed the `init_live_component()` twig function (**BC BREAK**) and replaced with the `attributes` variable which is always available (even if your component doesn't use `HasAttributesTrait`). To upgrade, replace all calls to `init_live_component()` with `attributes`: ```diff - <div {{ init_live_component() }}> + <div {{ attributes }}> ``` Commits ------- d638782 [Twig][Live] add html attributes system
kbond
force-pushed
the
native-attributes-1
branch
from
ce4d954 to
53ec97f
Compare
…r if the value changed (weaverryan) This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Live] Fixing bug where inputs would not re-render if the value changed | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Tickets | Fix symfony#144 and Bug E on symfony#102. | License | MIT ~~NOTE to self: I'm considering reversing this PR as it has some side effects where we lose input values on re-render. That can be solved with `data-live-ignore`, but it may be better to reverse this, and "fix" the original problem by telling the user to add a `data-live-update` attribute (or something like that) where we opt INTO re-rendering. There is basically a situation where we don't know if the user will want a form element to re-render (and update the value) or not. And so, the user needs to choose. The question is, which way should the "default" be.~~ For example, if an input rendered initially empty, then you typed into it, and, on re-render, the value was set BACK to the original, the input would *not* update on re-render, and it would be stuck with the old value. This was because the old input node and new input node were seen as identical... and so morphdom didn't bother to update that node. However, in reality, the value property of the "old" input had been changed and would no longer match the value "attribute" of the incoming input. Effectively, the 2 inputs had different values, but this difference was not caught by the system. Commits ------- a27fdd3 [Live] Fixing bug where inputs would not re-render if the value changed
This PR was merged into the 2.x branch. Discussion ---------- Rebuild js dist files | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Tickets | N/A | License | MIT Tried to update my project deps to use 2-x.dev of LiveComponent, but now that renderer returns HTML instead of JSON, it fails, because the LiveController still uses JSON. This rebuilds dist files from symfony#226, symfony#245, symfony#247 changes. Commits ------- 6906cc6 Rebuild js dist files
This PR was squashed before being merged into the 2.x branch. Discussion ---------- Ensure JS dist files are current in CI | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Tickets | n/a | License | MIT This action job will fail if the dist files need to be rebuilt. [Failure example](https://github.com/symfony/ux/runs/5022158414?check_suite_focus=true). Commits ------- 952725c Ensure JS dist files are current in CI
…erryan) This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Live] Re-extracting form data after submit | Q | A | ------------- | --- | Bug fix? | yes | New feature? | yes | Tickets | Addresses part of symfony#221 | License | MIT This fixes 3 bugs at once: 1) This recalculates `formValues` after submit, in case the submitted data *changes* the form's data or underlying structure (e.g. `CollectionType`) 2) If a field is suddenly missing entirely from a form (hint: think removing an embedded CollectionType form), then previously `validatedFields` did not "forget" that field. This caused edge-case bug when removing an embedded form that was validated, then adding a new form a moment later (which would be empty, but now validated). 3) When passing a `FormView` into your component that was already validated, on the first re-render, the validation would be lost and only modified fields would be validated. Ping `@Lustmored` and `@norkunas`. This mostly addresses `@norkunas`'s problems with how `getFormValues()` works, but this was also causing trouble for `@Lustmored`, potentially for different reasons. So, this may not solve the problems addressed in symfony#221, but it's certainly related. Cheers! Commits ------- c8b3a28 [Live] Re-extracting form data after submit
This PR was squashed before being merged into the 2.x branch. Discussion ---------- Cache deps where possible | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Tickets | n/a | License | MIT Commits ------- e3e2d84 Cache deps where possible
This PR was merged into the 2.x branch. Discussion ---------- Renaming Fixture/ -> Fixtures/ | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Tickets | None | License | MIT Minor: "Fixtures" is more standard, and it's also already configured in the php-cs-fixer config: https://github.com/symfony/ux/blob/6715a03c844e988c1ab6337b0605f88f1fcfe879/.php-cs-fixer.dist.php#L25 Commits ------- 6772737 Renaming Fixture -> Fixtures
This PR was squashed before being merged into the 2.x branch. Discussion ---------- Send data back and forth as JSON | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Tickets | Fixes symfony#251 (a bit unrelated, but it fixes it) | License | MIT tl;dr the "data" of your component is now sent via JSON for the Ajax calls. Ajax calls are still made via `GET` (unless you're triggering an action or your data is too long) with a new `?data={... json}`. **Longer Explanation** Until now, I've been trying to send data back to the server as `?query` params or as POST data (which is just query params stored in the body of the request). However, using query parameters simply isn't robust enough. Most notably, it makes handling `null` values impossible. If you have a LiveProp - e.g. `firstName` - on the server set to `null`, how should that be sent back to the server? As `?firstName=`? In that case, how do you distinguish from an empty string? One option is to NOT send `firstName` at all on the Ajax request. But then, what if you have a `LiveProp` that is an array with `['firstName' => null]`? If we don't send `firstName`, the array will dehydrate to an empty array. In the end, the goal is to effectively "freeze" your component's data, send it to the frontend, and have the frontend send it back. Using query parameters simply isn't robust enough to do this without, but JSON is. There is no real downside to this: it just felt more pure to send data back and forth as query parameters and return HTML. We still return HTML, but sending JSON will make the behavior solid. Commits ------- 2b808ea Send data back and forth as JSON
This PR was merged into the 2.x branch. Discussion ---------- [Live] fix build-packages.php in CI | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Tickets | n/a | License | MIT `build-packages.php` wasn't called correctly. Commits ------- e18f3e6 fix build-packages.php in CI
kbond
force-pushed
the
native-attributes-1
branch
from
92c6de9 to
dfdbb3a
Compare
This PR was merged into the 2.x branch. Discussion ---------- [Live] refactor AddLiveAttributesSubscriber | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Tickets | n/a | License | MIT Cleanup from symfony#220, this subscriber should not depend on the twig runtime. Commits ------- 2d429b0 [Live] refactor AddLiveAttributesSubscriber
kbond
force-pushed
the
native-attributes-1
branch
from
dfdbb3a to
2bcbc62
Compare
…ue (ie autofocus) (kbond) This PR was merged into the 2.x branch. Discussion ---------- [Twig][Live] null attribute values render without value (ie autofocus) | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Tickets | n/a | License | MIT Ref: symfony#220 (comment) A null attribute value renders without the _value_: ```twig {# templates/components/my_component.html.twig #} <input{{ attributes}}/> {# render component #} {{ component('my_component', { type: 'text', value: '', autofocus: null }) }} {# renders as: #} <input type="text" value="" autofocus/> ``` **TODO:** - [x] `null` vs `''` (symfony#220 (comment)) - [x] attributes without a value like `autofocus` (symfony#220 (comment)) - [x] LiveComponent dehydration/hydration issues (`null` values are lost during re-render) - solved in symfony#264 Commits ------- c1e484b [Twig][Live] null attribute values render without value (ie autofocus)
kbond
force-pushed
the
native-attributes-1
branch
from
d79f435 to
37acff2
Compare
kbond
pushed a commit
that referenced
this pull request
Nov 6, 2022
# This is the 1st commit message: WIP heavy refactoring to Component Initial "hook" system used to reset model field after re-render Adding a 2nd hook to handle window unloaded reinit polling after re-render Adding Component proxy # This is the commit message #2: fixing some tests # This is the commit message #3: Refactoring loading to a hook # This is the commit message #4: fixing tests # This is the commit message #5: rearranging # This is the commit message #6: Refactoring polling to a separate class
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is similar to #1 but built on top of #2. It purely hard-codes the attributes as "a thing all components have". This can act as a litmus test or possibly an escape hatch solution if our other ideas get insanely complex.
I think an argument could be made for this: attributes are already hard-coded just by the fact that
ComponentAttributesexists in ux-twig-components. We have had a hard time coming up with a realistic scenario (other than attributes) that would require some of the extension points proposed by our other ideas. This does leave the door open for a future extension system that attributes could take advantage of if/when it exists.