Seems like this was merged by lio above.
Added related 📌 Make providing customized CEs easy Active
Adding some proposal. I think it's not necessary to prefix all content entities with "drupal-" - that makes things to verbose and content-entities names are just a hand-full in Drupal sites, so those would be known and associated with drupal entities easy enough. The drupal- prefix would be there and used for everything which is not an entity, thus more drupally.
yes, existing projects would have to follow, but only once the new naming pattern is adopted. with CE-UI configuration projects should be able to use whatever naming patterns are desired. still, we should define good defaults.
as I understand this happens when using views field formatters, that's not supported. thus setting as duplicate from https://www.drupal.org/project/lupus_decoupled/issues/3461874 🐛 Custom elements Views: Per-field formatters are not supported/working Active
If I understood that wrong, please re-open.
I dont't think we support individual views-field-formatting, you may just take over the whole entity-row rendering to go via custom-elements entity-rendering, and from there on you can use CE-UI to format individual fields.
>So no matter which formatter I set, the field is always shown as target_id when checking the view.
I see, I guess expected behaviour would be at least to get the traditional-rendered drupal markup, right?
Tested it also, works great!
Makes totally sense, there is no reason to forward that content-type header - moreover it makes totally sense to NOT forward it. Thus let's go with that!
Great to see this land!
I can totally see that renaming makes the form workflow complicated though. Let's make sure we have a good, consistent form reload/rebuilding workflow so it works reliable across reloads/ajaxRebuilds.
// Remove EntityForm::afterBuild(), which calls copyFormValuesToEntity();
// we cannot update $this->entity before form values are properly validated.
// Any other validation functions must take this into account.
if (!empty($form['#after_build'])) {
$form['#after_build'] = array_diff($form['#after_build'], ['::afterBuild']);
}
hm. This sounds like the standard entity building and validation workflow, but that's 100% not the case. A lot of time has been invested into that, such that entity-building works and is independent from validation. It should give you a copy of the entity, and (IF I REMEMBER RIGHT) not update $this->entity, for that the submit-entity phase is done. So this seems weird. Can you spot more light on it why it's needed?
You need to watch out that $form object and this $this->entity does not end-up in $form though, this messes things up since the $form array gets cached. be sure to use "::method" notation for callbacks.
* @param string $validate_component
* (Optional) Only make sure the basic properties of the specified component
* are updated, in order to be able to validate the full component's config.
* Do not copy the component's full (unvalidated) configuration yet; leave
* other component's values in an undefined state.
*/
protected function doCopyFormValuesToEntity(EntityInterface $entity, array $form, FormStateInterface $form_state, string $validate_component = '') {
This is not a good idea imo. The typical workflow is to build a new $entity based upon form values, then run validation on it. That's problematic for us since this means running ::submit() phase on (CE-)formatters before validate and core-formatters are used to not having that. So to me it seems we should simply not have it updated. What's the reason for the partial entity update?
With the new dedicated CE-formatters for entity references, this would the best default. But then it's not clear which settings to default to, so sticking with "auto" is good enough. Let's rather make the UI to change it easy to use.
Not sure this is important or take care of by core-rendering? we can follow what core does here, but not so urgent imo.
We have discussed this problem and figured following the following logic should be all that's needed:
* When checking layout-builder for being enabled:
* When current view-mode (e.g. teaser) has no CE-display, fallback to default core display-mode for layout-builder setting-check
* When current view-mode (e.g. teaser) has a CE-display, NEVER fallback to core display-mode for layout-builder setting-check.
* If there is no core-display-view-config for the view-mode (e.g. teaser), take layout-builder to be disabled.
That way, the fallback for the layout-builder-check will only be applied when there is a fallback at CE-display config-level active. This is the naive assumption a site-builder would have on how things should work, so this is what we want to do.
the module should ship with the config for the thunder paragraphs, but I don'T see any here: https://git.drupalcode.org/project/custom_elements/-/tree/3.x/modules/cu...
so it seems this is not done.
If quick and easy (I really don't know yet): add backward compatibility loading for already present 'alpha1 style' configuration. (That's being used in several places in an internal project.)
No, let's not spend efforts on BC with an alpha release. It was marked to be not ready for prime-time. Let's break it once now and move on.
seems to be the main blocker atm
discussed with roderik, we want to do
https://www.drupal.org/project/custom_elements/issues/3455496
📌
Drop simple and single+simple slot normalziation styles
Active
So setting the evalution to done.
I don't think that's worth it. IT's conceptualyl complicating things A LOT for some special cases because of BC reasons. With the new model, we most likely would have built it differenty simply. For those special cases, auto-processing and custom processors is still doable.
Let's not do this until we 100% see the need for that.
I pushed my backend code at 3336148-forms-simplified - it's largely simplified. I need to add the docs still though.
brianperry → credited fago → .
Slot normalization styles are used to simplify the generated JSON content format of a custom elements tree. Atm, it's internally used and auto-activated when someone uses a setSlot() or setSlotFromCustomElement() call without an index (default). The result is that the e.g. a slot named "image" with a single custom-element for that image will result into a JSON object with the image-CE output being set as the property "image". Without slot normalization styles, it would be an array with a single entry, which then is the image-CE output.
The need for this came up to make the JSON output nicer when a lots of slots are used. However, since the creation of the module we moved more and more away from using a lot of slots. Slots are good for nesting arbitrary kind of (CE-) markup, but they are not needed when simply a nested data structure (like an image object) is to be created.
For the use-case of creating an image object, a custom-element is not needed, since when rendering a CE the FE does not not know what it will contain, so it loses some control. But usually for rendering images, the FE wants to stay in control and e.g. be able to adjust the image output or e.g. add some custom classes to it.
That said, the CE is not needed for this use-case. But when creating nested data structures with CE the rendered-markup becomes more readable than without. When creating nested data structures with attributes, a rendered markup would contain ugly json-serialized data. Thus, there is some advantage in that practice of using setSlotFromCustomElement() with single-valued CE for passing nested data. This is for NORMALIZE_AS_SINGLE_VALUE.
But then, we also support NORMALIZE_AS_SIMPLE_VALUE and NORMALIZE_AS_SINGLE_SIMPLE_VALUE, which does the same but allowing normalizations like headline: "text", such that there is no wrapping element any more. I'd even argument now, this is counter-intuitive to have a slot to be normalized in a JSON that is not an object at whole, because it would not be render-able via the regular helpers for rendering custom elements any more.
Thus, I think we should keep supporting setSlotFromCustomElement() or setSlot into a single valued CE, but we should drop support for simple and single+simple slot normalziation styles. Given that we should simplify the code to only support the single-valued slot-normalization style.
thinking about this once again, I don't think there is any good reason to deprecate this. Yes, this is somewhat unneeded API since the same can be achieved by creating a custom element and passing it as slot, but that's not as straight forward to do when you just want to create a slot with some html markup.
I think we can still support this, the code is there, it's not complicating things, so there is no reason to introduce a breaking change here really.
thx, looks great, merged!
I only had one comment, which could be added in a small follow-up PR if you agree.
Else, I'd suggest to add another small follow-up for introducing a new config option into the field/image formatter if the reference points to an image: An image-style drop-down.
That would be handy for media-references pointing to media-iamges as well, but I guess this should be its own follow-up.
Setting to needs-work for reading my comment and deciding how to continue!
as discussed we want to move this to a separate formater first
as I understand the problem is the configuration is not applied 1:1 as expected. yeah, we should allow setting some explicit '' prefix and simply do so when generating the element from config
thx, yes, we could introduce a new base class for tests, but nvm - main point is we have those tests now. Reviewed, fixed one comment and merged.
agreed. If a formatter returns
$configSubform = [
'settings' => [],
]
then the formatter shoudl be fixed to not return an empty form like this. The MR does this just fine, thus merged. thx!
Commented, please see remarks above.
Generally, change see to go into the right direction, but I dislike including the improvement of our ER-formatter here.
This should gets its own dedicated issue, so it appears properly in release notes. When doing so, it gets its own description why it is needed, and should come with some quick kernel test to demonstrate how it works correctly from a simple isolated API perspective. Maybe best, let's still have this issue, do it first, and then get back to finishing up here?
I figured the cache metadata was missing, added that and merged!
Thx! :-) merged!
commented, please re-check
alpha1 is there :-)
Thanks for bringing this home! When reviewing changes, I found an issue with it though. Created 🐛 Entity-CE-Display form incorrectly assumes every plugin has a settings-form key Active .
took a fist stab on this. it basically works already, but the ajax submission of the configuration form does not work yet. probably the entity-ce-display form has some problem there still.
thanks, works well, merged. thus, this point is still open, right?
- Empty plugin settings forms should not see a "settingsForm" edit icon
thx, good fix. Merged and setting back to needs-work for the main issue goals.
thx, merged!
ok, worked over it - as discussed with going with the semantics of "DISABLED" == not-existing as core does it usually also. Also extended test-coverage for making sure the fallback works correctly.
on this now
Some thoughts:
* We should check whether it would be enough to provide one CustomElementsFormController and a trait. less loptions, less confusion.
* Most logic seems to boil down to trait method
public function getCustomElementsContentResult(array $form): CustomElement {
return CustomElement::createFromRenderArray($form)
->setTag('lupus-form');
}
that we want to improve to render into a "drupal-form" component, which gets the data to render the
element, so it can gain some control over the form.
* By default, we should submit forms via the proxy, back to drupal via /ce-api prefix of the page. So when the route is ce-enabled, the form should be processed as ususal. Validation fails should work transparently that way. For that to work, we need to make sure the frontend correctly passes through POST requests. That should do away the need for all that custom-code for the user forms.
* For adding support for a form, a controllers needs to be provided which renders the form and creates the drupal-form CE via the helper like getCustomElementsContentResult() for the result. If form processing results in a redirect, lupus-ce-render should be able to handle that transparently.
@glynster: Thanks for your suggestion!
I'm not a big fan of adding something like nuxt-auth in the frontend, since it complicates the frontend stack, e.g. it becomes harder to support multiple frontends, upgrade nuxt majors, etc. But that said, nothing would speak against supportin this optionally. The advantage would be that it makes the frontend aware of permissions via oauth scopes, right?
Admin menu would definitely be a nice use-case, but in the end we only need a simple flag in page-api responses that tell us whether the users has backend-access/permission or not. Use-cases for permissions to check vary, so we could add a feature to allow permissions to be sent to the frontend as part of the page-api response. It smells a bit like a (simpler) rebuild of oauth-scopes though.
Maybe let's add a feature request and work on supporting that in a dedicated ticket?
As default, I think the cookie based access is fine, we just need to finish https://www.drupal.org/project/lupus_decoupled/issues/3336148 📌 Add support for forms Needs review to have user-login forms on the frontend working.
looks all good to me! however, the gitlab pipeline is failing with the changes. we need to make it accept it.
created https://www.drupal.org/project/custom_elements/issues/3446485 📌 Simplify entity-ce-display form and AJAX logic Active as follow-up
ok, I worked over the checkbox labelling and logic, please see
https://git.drupalcode.org/project/custom_elements/-/merge_requests/55/d...
I think we can merge this is as a first step.
> - Fix editing keys and pressing save bug
I took a quick look at this. This is not as simple as I hoped it is. Turns out there is some complex ajax-refresh logic going on, which is handled/implemented by the attached JS library "field_ui.js" ($form['#attached']['library'][] = 'field_ui/drupal.field_ui'). It seems this JS is atm necessary to handle updates of the region field when elements are moved into "hidden" and back to "content" region. This JS also somehow takes care of changing the submitted POST data to incude the "name" field.
let's also check whether we can drop this:
* \Drupal\custom_elements_ui\Plugin\Derivative\CustomElementsUiLocalTask::alterLocalTasks
* custom_elements_ui_local_tasks_alter()
seems unneeded to me.
checkbox: "Force rendering as custom element"
thx, slightly improved the text and merged it!
thx, changes work well, so merged.
But I think the sole button is not good UX. We need to explain the user why the button is there. Something like
"Layout builder is enabled for this view mode. Thus, the configured layout is rendered is getting rendered in a custom element. Configure the layout to customize how the element should be built."
Then show the button?
We won't change this anymore for 2.x - so moving to 3.x
it seems that a conclusion we came already on a year ago: there is a pre-existing ticket for this: #3323558: Make configurable Auto option build Custom element same as existing processors do →
I resolved conflicts with gitlab and merged results! Thanks!
Got it basically working. I've worked quite over the form. There are a few todos left, but I think it'S better to merge now, since we have something working, and do small todos in follow-ups. That helps to avoid merge issues with other MRs.
todos:
- Merge CustomElementsEntityDisplayFormBase into EntityCustomElementsDisplayEditForm
- Empty plugin settings forms should not see a "settingsForm" edit icon
- Fix editing keys and pressing save bug
so I guess we can simply drop all the CE-auto-generation now + have displayCE config be auto-generated and used on the fly by default. as BC, we could add a new flag in the displayCE config "forceAutoProcessing" which makes use of automatic processing for the whole entity, instead of applying the per-component configuration. Then everyone who wants BC can set this flag. We could even decide to expose it as some sort of setting in the UI later.
thx, this seems pretty good already. I dropped a few comments, please take a look!
thx. changes look good + tested it successfully.
WIP
Merged!
added some test-coverage to make sure config is passed through to the formatter correctly also. works now, MR is ready
quick draft
Took a stab on how it could look like in a fully BC way. What do you think about that?
https://git.drupalcode.org/project/custom_elements/-/merge_requests/49/d...
hmm, this is not so "optional" as I had thought of it.
Modifying all Processor classes can be done before updating the custom_elements module to version 3.x, without impact. Immediately after updating the module, all previously unmodified Processor classes will break.
That's not good, since it could result into severe upgrade headache. e.g. at lupus_decoupled we want to support custom_elements 2 and 3 in parallel until 3 is stable. Also that means custom code must be changed when updating, what makes it complicated. I think we should check and find a way to avoid this.
>The name should be optional, to keep BC for 2.x processors.
We really need this BC I think.
the way it works is that you simply put in the next upcoming version number in the change record, so it can be already be published. it should be published when the change is committed to the dev-version, so it's visibble for it. Thus, I added the version and published it.
Core field formatters store into entity-display component something like
component-of-field:
type: <formatter-id>
settings: <array-of-formatter-settings>
But core field formatters, they auto-generate some plugin configuration which looks the same. So their plugin configuration is something like:
configuration:
type: <formatter-id>
settings: <array-of-formatter-settings>
But in current 3.x branch our ce-field-formatters are having settings and configurations as same thing:
component-of-ce-field-formatter:
formatter: <formatter-id>
settings: <array-of-formatter-settings>
configuration:
formatter: <formatter-id>
settings: <array-of-formatter-settings>
custom-config: <value>
What is very similar. but our plugins can change configuration, not only settings.
Maybe we need to move it to clear separation:
component-of-a-formatter:
formatter: <formatter-id>
configuration:
settings: <array-of-formatter-settings>
custom-config: <value>
thx, all good. merged, thanks!
@roderik: Thanks, merged! Could you also add a change-record? See
https://www.drupal.org/list-changes/custom_elements →
We should document all breaking changes there
@roderik: Thanks, merged! Could you also add a change-record? See
https://www.drupal.org/list-changes/custom_elements →
We should document all breaking changes there.
finished step2 and made things work. Stumbled over quite some things, added lots of todos for them and created issues for 3.x.
Finally, I worked over tests and made them work again. Partially I added new display-ce configs to make things work, so we see them being taken into account.
Also, I had to work over the render pipeline such that it actually renders config first, and fallsback to processors second. Given that we should have the foundation for 3.x with this PR, but need to work through all of those smaller todos in follow-ups.