#1158 allow full-row field templates in embedded view #1170
Conversation
| @@ -59,7 +59,7 @@ export default function maEmbeddedListField() { | |||
| <div class="remove_button_container"> | |||
| <a class="btn btn-default btn-sm" ng-click="remove(entry)"><span class="glyphicon glyphicon-minus-sign" aria-hidden="true"></span> <span translate="REMOVE"></span></a> | |||
| </div> | |||
| <div class="form-field form-group" ng-repeat="field in ::fields track by $index"> | |||
| <div ng-repeat="field in ::fields track by $index" compile="::field.getTemplateValueWithLabel(entry)"> | |||
There was a problem hiding this comment.
The "form-field" and "form-group" classes here are redundant, since "ma-field" inside this div also creates a div with those classes. Having them repeated here interferes with hiding rows based on entity values.
| @@ -223,6 +223,11 @@ When displaying a post: | |||
|
|
|||
|  | |||
|
|
|||
| Caution: If you want to use [`Field.template()`](./Theming.md#customizing-the-template-for-a-given-field) | |||
There was a problem hiding this comment.
This paragraph is not clear to me. What do you mean? :)
There was a problem hiding this comment.
The docs for the "Field.template()" config in Theming.md suggest using it with an "<ma-field>" tag with 6 "magic" attrs that must be quoted verbatim, i.e. (ignoring the "ng-if" here)
.template('<ma-field ng-if="entry.values.category" field="::field" value="entry.values[field.name()]" entry="entry" entity="::entity" form="formController.form" datastore="::formController.dataStore"></ma-field>', true)
Those 6 attrs expose the implementation details of ng-admin and they must be gotten exactly right for things to work. To make this even harder for users, the magic attrs that you must quote are different for "regular" fields to "embedded" fields.
Actually fixing this is out of scope here, but I thought users might benefit from a warning about this, in case they try to use "template()" on an embedded field with the recipe from "Theming.md" and can't figure out why it didn't work.
There was a problem hiding this comment.
Right: we should think to fix this black magic. 👍
Can you add your explanation directly into the README?
There was a problem hiding this comment.
i'd be happy to, but I'm not quite clear where you want this explanation. or what text you want in addition to what I've already put, sorry. Perhaps you should add an explanation that makes sense to you directly to the code? Or let me know in more detail what changes you had in mind, and I'll make them.
(Also perhaps this issue about template() would be better addressed on its own Issue thread, as it's separate from the problem in #1158)
| @@ -223,6 +223,11 @@ When displaying a post: | |||
|
|
|||
|  | |||
|
|
|||
| Caution: If you want to use [`Field.template()`](./Theming.md#customizing-the-template-for-a-given-field) | |||
There was a problem hiding this comment.
Right: we should think to fix this black magic. 👍
Can you add your explanation directly into the README?
|
Are you waiting for something from me on this? From my perspective, this is ready to merge. It doesn't fix the "black magic", but it's a step in the right direction. I haven't added any unit tests, as I feel like #1158 will be really difficult to demo in a unit test. Is that a blocker for merging this? |
|
@RichardBradley I'll take a look at both your PR and issue this afternoon. |
|
Ok, it's a tough one and I'm not sure that your fix is optimal. |
Perfect is the enemy of good :-) It's not optimal, but it's better than what's currently there and it doesn't make anything worse. I have been using this fix in production since shortly before I posted it here. I would suggest merging it while you work on an optimal fix in parallel. It's up to you, of course. I can continue to use a forked version with this fix applied if you don't want it in here. Thanks for your time and hard work on |
|
(From my point of view, this is ready to merge) |
This fixes #1158