🇳🇱Netherlands @roderik

@fago

* MR 93 had a branch in the origin repo. After pushing the above commit (including a phpcs fix), I deleted the branch which closed the MR.

* Opened a followup issue for creating the test later. (Juggling priorities before RC1.)

roderik → created an issue.

reproduce the above (serialization of the markup format)

This seems way easier than I thought; it's basically the most simple version of a setSlot() call, and then serializing that.

I'm just not experienced enough to know, yet. (Anything that is called '*Slot*()' is still magic to me.)

I'll change course and commit this now, do a new beta release so people can work with probably-non-broken code, and think about a test later.

TODO ON ME: check if tests for JSON serialization can be easily added to this module or if e.g. lupus_ce_renderer is a better place.

custom_elements_ui isn't actually enabled. See https://git.drupalcode.org/project/lupus_decoupled/-/merge_requests/95/d....

(I didn't want to open a new issue, that would IMHO get more confusing.)

Hit-and-run thought: per #3440446-11: Add prepareBuild step similar to entityprepareView →

(If it's just a question of doing a loadMultiple, then fine - but the example code contains access/caching magic that I don't want to get stuck in, before handling some other priority issues.)

...it can't be just a question of doing a loadMultiple... right? Because if that were the case... we would surely not need to do that prepareBuild()?
We might as well do that in build(). (That also makes it easier to, in theory, implement a memory-saving option that loads the entities in batches.)

* offer two methods, one which we call generate() and works as the one in the MR, minus the empty string langcode thing.
* another one, which we call buildEntity() and buildEntityMultiple(), or buildEntityContent() which works like generation, minus the translation context.

I now implemented this:

• generate() - works as before (does language translation) except also has $account
• generateMultiple() - works as previous commit, minus the empty language string (always translates)
• buildEntityContent() - works on multiple entities, does not translate

There's no "single" equivalent of buildEntityContent() ; imo it woud not make things easier to understand (or code).

Imo, this terminology introduced by the CE-formatters of "Builds an entity's content as custom element." is fine and should apply to the whole render-process, i.e. including layout builder. Thus, it should include everything, minus the language-negotation. Then we have that re-usable utility also + shortened the code.

The same way, I'd find it logical that the ce-prepare-build hook and a likewise ce-entity-build hook fires in every case of our 3 rendering variants (force-auto, layout-builder, formatters).

Yes. This last thing was already the case. Hence the large size of the outer foreach()es.

But I've 1) split out some code to make the flow a bit better, 2) implemented your suggestion to make 'force-auto' a more distinct piece of code.

Per yesterday evening (before the MRs), I had trouble reproducing the screenshotted situation and doubts whether the arrows even represented slots. I don't understand this serialization code enough.

Current issue status from my perspective:

I'm guessing it was introduced by 📌 Drop simple and single+simple slot normalziation styles Active .

While I can't reproduce the above, I will try to verify by reproducing 🐛 JSON serialzation of markup only slots is broken Needs review (testing user login form).

This needs a test to make sure a bug like this does not sneak in again. After reproducing, I hope to be able to create a test and add it to https://git.drupalcode.org/project/custom_elements/-/merge_requests/93.

I'll also try to update our internal project to the latest custom_elements version, so I can run the associated tests, before committing this. (Per the README, this still needs us to create/check our own update path; I expect/hope that will be relatively quick.)

This took longer than I expected. Yesterday's code wasn't good, because it was also prepare()ing entities that don't even go through field-based processing. New one is up now; the single added method is... long, but IMHO readable.

specifically on the one comment I left, for the specification/use of the $langcode method parameter. I trust the rest of the code enough to commit.

I have created a followup for actually using this in entityreference formatters etc. (If it's just a question of doing a loadMultiple, then fine - but the example code contains access/caching magic that I don't want to get stuck in, before handling some other priority issues.)

I created a draft CR for the removed methods. (They only existed for a few months, but it's some record at least.)

roderik → created an issue.

roderik → created an issue.

This is a first request for review, to see if we agree on the new call structure:

• There are now two split-up 'multiple' phases, and they are now called from the one caller that would potentially benefit from speedups
• but I haven't added the code in our Entity-Reference CE-Field-Formatters yet, that would provide actual speedup. (I am stopping for the day. Potentially this could also be done in a followup.)

Relevant public methods in CustomElementsGenerator:

• generate(ContentEntityInterface $entity, string $viewMode, string $langcode = NULL)
  • I think this needs a 'multiple' version.
• buildEntityContent(ContentEntityInterface $entity, CustomElement $element, $viewMode)
  • did not exist in 2.x branch. Isn't called from 'external' classes yet.
  • doesn't do much, nowadays: just creates a CE display and calls buildCeDisplay()
• buildCeDisplay(ContentEntityInterface $entity, CustomElement $custom_element, EntityCeDisplayInterface $display, string $viewMode, AccountInterface $account = NULL)
  • did not exist in 2.x branch. Isn't called from 'external' classes yet.
  • contains the logic we want to split up.
  • has an extra $account argument.

I believe that we can

• add generateMultiple(); generate() and generateMultiple() are going to remain the 'simple' entry points.
• deprecate both buildEntityContent() and buildCeDisplay()
• add for the 'advanced' callers (and call from within generateMultiple()):
  • prepareBuildEntities(array $entities, string $viewMode, AccountInterface $account = NULL): EntityCeDisplay[]
  • buildEntities(array $entities, array $custom_elements, string $viewMode, AccountInterface $account = NULL): void

Thank you for the details. This is as I had expected - though I had not checked it with you -- and I would not have known to look in EntityReferenceFormatterBase::prepareView() for the partial-entity-loading code.

Working on this.

Done:

• Created followup 📌 Add test for single-slot normalization style Active
• Created draft CR. Just said "there is no replacement", as I cannot infer from just the code in the normalizer, how things exactly come out / whether there is a way to get the exact same result.
• Implemented requested changes (with one comment just to be sure, because see previous point).

roderik → made their first commit to this issue’s fork.

roderik → created an issue.

Just tested: there is nothing special about Core field formatters here. Any settings in the "configuration" array are not 'cleaned up' / stick arouond, when we change the formatter type from one type that has a certain configuration value, to another type that does not.

We should clean this up, but it doesn't feel super urgent to me. The unnecessarily-saved values don't get in the way of anything; they are just ugly.

So I don't think we have to do this before stable release.

@fago for review, as this is a basic change to your code.

Better comments for myself / later reference (I will be able to find them back):

g/setSetting(s)() + static defaultSettings() are from PluginSettingsInterface

g/setConfiguration() + non-static defaultConfiguration() are from ConfigurableInterface

settingsForm() & settingsSummary() are not in an interface. Core field formatters have them in FormatterInterface but we don't implement that.

settingsForm() is not used anywhere, can be deleted. settingsSummary() we should probably add to CustomElementsFieldFormatterInterface. (We've done the same with static isApplicable() which ia also in FormatterInterface.)

build/validate/submitConfigurationForm() are from PluginFormInterface

There's a Core issue to merge PluginSettingsInterface into ConfigurableInterface ( #1764380: Merge PluginSettingsInterface into ConfigurableInterface → ) but it isn't going anywhere. (If that is ever done, I assume all "*Setting*" names will be deprecated.)

The only reason why we implement PluginSettingsInterface, seems to be that EntityDisplayInterface:getRenderer()'s return value requires it. Otherwise we would not have these duplicate methods.

We could have chosen to not implement ConfigurableInterface instead, but we didn't. (Also PluginSettingsInterface::defaultSettings() is more clunky because static.)

@useernamee thank you, you're right.

Some fields pass a BaseFieldDefinition class, but some fields pass a FieldConfig class.

(And all of these have a getSetting() that should contain whatever a specific formatter needs. I just needed to wade through some TypedData stuff and Plugin stuff, to be able to verify that my application code can work with this.)

So this issue is 'works as designed'.

This now pretty much depends on [#3472359:].

Marking getRenderer() internal is one of the 'sweep up tasks' in 🐛 Update path and documentation Active .

For completeness: I did work on followup tasks in 📌 Simplify entity-ce-display form and AJAX logic Active , but the cleanup didn't happen there.

I totally admit to not having a detailed step-by-step plan until now, because there were too many unknowns for me in the current code. I now did a better writeup of what my plan was, in 📌 Ability to add a field multiple times Active -- which is now slated to be the "cleanup / followup issue".

@fago I understood from you outside this issue, that we don't want to have ✨ Allow configuration of static props Active be a 'stable blocker'?

So I moved it into "Before or right after being stable" in a new group 3, and I added a new prerequisite 📌 Ability to add a field multiple times Active before it.

roderik → created an issue.

You're right. I moved the "delete dropdown" to the right.

Things are working now. I think I tested everything.

Done:

• The is_slot duplication is done (by adding it to the array_diff_key() in EntityCustomElementsDisplayEditForm::copyFormValuesToEntity())
• 'region' is (likely) removed in 📌 Simplify entity-ce-display form and AJAX logic Active , along with the empty 'hidden' key/li>

Left to do: re-testing the change from a 'core' formatter to e.g. "raw" and seeing if the 'settings' are gone.

@alexpott / @useernamee

The 'v2 way of rendering' (Processors) is still the same in v3, so there should be no need to test on v3. We should even be able to test these issues (that don't trigger any use of v3-style Custom Elements Displays) by temporarily copying v2 code back over v3.... for the moment at least.

I'm just noting this befause updating to v3 is explicitly non-trivial.

If possible / for the moment, any fix that comes out of this will be applied to the v2 branch also.

Current status of the "3446485-ajax" branch (rebased on current 3.x and tidied): Two out of three points in the previous message are implemented:

• untied Core AJAX functionality from the "name" + "is_slot" row elements, so those bugs are gone.
  • I needed to clone the field_ui JS library for this, into the module - just to make one little change. But I guess we expected that already.
• I did keep the "region" dropdowns as a means for deletion, because anything else would have required hacking tabledrag.js as well. IMHO this is "good enough" and if we want an actual delete button, that would be a post-stable-release followup.
  • The "region" selector is now always shown on screen, and renamed to "Delete?". See screenshot.
  • "region" is removed from the config schema now. I think this requires no CR because this is auto-removed on save, and we never did anything with the value anyway.

Left to do: the (AJAX) functionality for adding a field to the display.

I fixed a glitch in the #states situation (see 📌 Improve ordering of UI elements Fixed ) at the same time.

roderik → created an issue.

There are now PHP warnings on the screen if Layout Builder is enabled, because the following code wasn't moved down:

    if ($layout_builder_enabled) {
      $form['force_auto']['#states'] = [
        'visible' => [
          ':input[name="force_layout"]' => ['checked' => FALSE],
        ],
      ];
    }

I will move that down too.

roderik → created an issue.

Created followup ✨ Move layout builder code into processor service Active for the idea discussed.

Created CR. It's very small and I don't think it needs extra info. So, for this time: closing this issue already. (If I see that CRs need more review, I'll change that practice.)

roderik → created an issue.

This ticket is an unfortunate mess, at the moment. I have two independent branches:

1.

form-rows-as-components branch. We need to move away from "one row in the form == one field", because in the new paradigm enables us to add one field several times (with different formatters) and we want to support things like ✨ Allow configuration of static props Active . This branch will also clean up much of the messy comments / temporary code in the form, that was working towards this situation.

I however shelved this for now. I'm at the point where the basic properties in the form (name, formatter) rebuild and save correctly, but the plugin settings form does not show.

I will likely need to re-index the plugin settings form from "field name" to "new row name" too, and I'm not sure what issues I'll encounter there / how long they will take.

So I shelved this, and will continue working on this later (probably in a followup issue that is going to be a prerequisite for ✨ Allow configuration of static props Active if not #3446287 itself -- maybe titled "allow a field to be added multiple times").

2.

3446485-ajax branch. Form row names are still named after the fields.

• I hacked some more temporary helper code in order to build only the fields in the "Content" region (instead of all of them, as the Core form does
• (This requires EntityDisplayFormBase::form() to be copied wholesale into the form, and then changed)
• (and will very likely require a modified 'field_ui_table' form element, so I copied the FieldUiTable class to CustomElementsFieldUiTable, to be freer in hacking away the field_ui JS)
• ...and only the fields which are not in those rows, can be aded using the "Add" AJAX functionality.

However, I have not yet done anything substantial yet, like

• added an AJAX callback to the "add" select element
• untied Core AJAX functionality from the "name" row element (which should not have a callback, it just causes a bug, per the bold warning that is now on the screen)
• check how to implement the deletion. (Deletion is possible currently by explicitly selecting "Disabled" in the region dropdown, and then saving. That is obviously not good UX.)

I have 60-90 minutes more to hack on this before I'm yanked away, and then..... we'll need to plan next steps for the coming 6 days while I'm basically gone. Or: not plan, and I'll pick this up immediately as soon as I can / as I return.

I made a mistake:

The "Save this screen first" text next to the Manage Layout button is always visible, even if the Manage Layout button is not.

I don't think it's super important how this looks, so I am going to just wrap both the button and the message in a fieldset (to make the message hide-able at the same time as the button).

Install hook goes here, per the description. (Check if the module is actually present, because it can still be v2.)

We should't do the install hook here. New people will decide for themselves, whether they enable the uI module.

(We are doing the update hook here,

• partly to make 📌 Auto-enable Custom Elements UI when available Active because we have no good unambiguous place for an update hook there.
• and because realistically people updating from 2.x will always need it anyway.

Implemented requiested changes (Rename forceLayoutBuilder; implement requested description changes.)

As discussed offline: we're going for the new proposal.

It definitely makes the UI more intuitive.

Now for review.

I'll make a CR.

Reminder to self, to make CR (also for 🐛 drupal prefix is added to configured custom element names Active ).

Release will be done hopefully very soon.

Yes.

This whole (hastily created) issue's premise was wrong. Because $this->originalMode is not the 'original view mode of the config object. It is the 'originally requested view mode'. (Naming is bleh.)

So $this->originalMode is actually what we want here / the proposed fix is bogus.

Instead, @fago's recently added createCopy() fixes things. Today I learned-or-relearned about createCopy() / what happens when a non-default view mode is activated through the UI.

Thanks for testing.

Thanks! Merged, after fixing a failing test (I guess it was just a typo) - and created 📌 Doublecheck CustomElementsRenderMarkupTest results Active for later.

roderik → created an issue.

If you want to have the README changes moved to a separate PR, I can do that.

If you want to have the test moved to a separate PR, I can do that.

The code changes are not huge.

There is no update path in code.

The MR does not re-add a warning about the UI being buggy, it just moves it out of an 'if' construct.

1. Since this ticket was created, something important changed: We now always have a CE-dipslay-mode config, since the object (not config) is auto-created when missing. So the case "config is missing" is nothing we need to care about any more.

I do not think anything changed since this ticket was created.

The case "config is missing" == "the CE display is not enabled/visible in the UI" == "the default behavior of an auto-created CE-display-mode" is a thing. We need to define it, no matter what we call it.

Thank you for outlining your new thoughts, now I can think about it and summarize from my point of view:

In your original proposal, as implemented by me:

• If a CE display "exists / is visible in the UI", we never check layout builder.
• If a CE display "is missing / is not visible in the UI", we check if LB is enabled and act accordingly. If it is not, we fall back to the default CE display.
• Consequences:
  • It is impossible to build using LB, when specifying the "default" view mode. This is strange, but IMHO not bad: you can still build using the "default LB configuration", by specifying any view-mode (really any random string) for which no CE display exists.
  • The fact that LB is enabled for view modes which are not visible in the UI, is not apparent. (This is the comment that worried you? But it is a direct consequence of the proposal.) We can 'solve' this with an UI message saying "Layout Builder is enabled for X, Y and Z view modes" when showing the default CE display.
  • People need to go to the core-display screen, to enable Layout Builder.

In your new proposal:

• If a CE display "exists / is visible in the UI", we have some extra checkbox / config-option to
• Consequences:
  • We have an extra config option on the CE display object, which can go out of sync with the LB settings. (We need to accommodate for that config option being true, while LB does not exist.
  • A CE display object needs to be created solely to have this single config option be enabled, because it's the only way to use Layout Builder now.
• If a CE display "is missing / is not visible in the UI", we never check layout builder and always fall back to the default CE display.

I still like the original proposal because I don't like unnecessary/duplicate saved config info, but.... more importantly, I thought that YOU did not want this extra checkbox, from the beginning. Which is why I implemented the current solution, according to your spec.

I do not see the "Consequences" as a big deal.

I will add the following response, just for completeness / in case there was a misunderstanding:

2. Thinking about use-cases again, I think it might be a valid use-case to enable layout-builder on the core display mode, but to not use it on the CE-API output.

Obviously, yes. In your original proposal, this is always the case when a CE display "exists / is visible as a tab in the UI".

OK. I'll make sure to do this, when getting to this issue in kind-of-roadmap-order.

@fago Thank you for the message. I agree that this is clearer, but had to see this in practice to really get a feel for it.

We have implemented something a while ago in our internal projects, because a fix was urgently needed for the above "Problem/motivation". It works well in practice. Now, tests are added to prove out that it works 100% as I thought, and I can verify things more easily if we need to discuss/change more.

The MR also includes changes to the README. I need to do this now, otherwise I'll forget to include details later. Since this change also has effect on the update path (makes it more straightforward), I have documented implications for the update path too.

However, there is a complication with this proposal vs. the fix we are running internally.


* 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.


Point 2 is fine.

But the indented 'point 3' (as I read it now) seems to refer to 'point 1, not 2'. Which I didn't understand until much later. So I have to doublecheck:

Do you mean,

• If e.g. teaser has no CE display
• and teaser has no core display-mode for layout-builder setting-check
• then do not check "default" core display-mode for layout-builder setting-check, but instead immediately assume that the default CE should be used?

There are some complications with that.

First:

It will be impossible to use layout builder with the "default" view mode... unless a CE display for "default" view mode is explicitly created and disabled. IMHO we should heavily discourage this, because it is not intuitive. The "default" core display-mode also cannot be disabled, in practice. There is no UI for this (and a few months ago, you explicitly reverted a change where I made the default CE display disable-able - which I now agree with). We haven't yet tested how our UI behaves with a disabled default display.

So, in summary: it will be practically impossible to use a layout configured for the "default" view mode, to build CE displays. Which may be an issue in itself(?)

But also - secondly:

The update path will be more complicated for our internal project. And therefore, I suspect, also for other existing sites. It will not be "if you want no hassle at upgrade time, just execute this code" anymore. (See README)

We have several entities/bundles that are rendered in "full" view mode, and expect to be built using a Layout, which is set up at the "default" view mode.

This will stop working. Therefore,

• During the update to v3.0-stable, we will need to take that layout from "default" view mode and copy it to "full" view mode.
• (Note if we were using this layout to also render non-custom-elements, we would need to keep the "default" layout too. Or change more code.)
• Or alternatively, we will need to explicitly disable the "default" CE display. But we don't want that in 'normal' sites (see above), and it can influence other non-default view modes too -- leading to more auditing of our sites.

Because of this reason, I'm going to push back a little here, propose to also check the default core-display with a changed first-bullet-point:

• When current view-mode (e.g. teaser) has no CE-display, first check the relevant core display-mode (i.e. teaser if it exists/is enabled; otherwise the 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.

This is the naive assumption a site-builder would have on how things should work.

I'm not totally sure about that.

When a site builder sees the "Manage Custom Element" screen, and the CE display for "teaser" is not enabled, then they see no information at all about "Teaser". There is no "Teaser" tab. So they also don't see that Layout Builder is active for "teaser".

So they will assume that the "Default" CE display is used, instead. They will not think about core display-modes.

I believe we have to tell them that this is not the case (but layout builder is instead enabled for view mode "teaser"), in an explicit extra message.

I believe we can/should do that, regardless whether we also check the layout builder setting on the "default" core display-mode. So there's not a huge difference for the naive site builder, whether or not we also check the "default" core display-mode.

---

I created a MR and set it to "needs review". What is done, is the code (according to what our internal projects are running now == my proposal), the tests, and the README changes (describing the "my proposal" situation).

What is not done yet, are the messages in the form which point to Layout Builder; there's still a @todo in the code.

I believe this is a duplicate of 💬 Force HTTPS for ACS and SLS Closed: works as designed .

There's a pointer in the samlauth README now, and a pointer to the Drupal documentation page https://www.drupal.org/node/425990 → .

I believe configuring these 'reverse_proxy ' settings is better than solving things in the samlauth module, because 'Drupal thinks the site is on a wrong port/protocol' affects more than just SAML. It's a general Drupal issue.

If you can't make it work, then I am open to discussing this (like in 💬 How to resolve wrong protocol issue (reverse proxies with dynamic IP)) Needs work ), but I need specific info on why it would not be working. Until further notice I believe you're getting the port/protocol info in some proxy headers that Drupal can pick out,

If you have suggestions to improve the README to make this more obvious, I'm open to them.

The Drupal documentation page really needs an overhaul / to be more prominently featured. Realistically... that's not likely to make it to the top of my to-do list.

Making "active" again just so the title shows up in the list.

(I closed this 'Feature request', but actually I tend to keep many 'Support requests' open and showing in the list, until that time I might make a FAQ or reference on the project page.)

Thank you for this. It looks good. I tested.

Random blabbering:

I needed to remember what the structure of these properties was. There are:

• Properties like "format" and "key_size"
  • are extracted from the key itself (which is likely referenced in the key_provider_settings, as e.g. an environment variable or file name
  • are therefore not independent, and we don't really lose info if we don't store them.
  • but the README gives an explicit example of using them for e.g. selecting a specific key. So it makes sense to export/re-import them in config.
  • are optional: the user can enter an unrecognized key/cert and check the "do not validate" box. But nothing config-related is complaining if these properties don't exist: good.
• Property "comment" (and maybe I missed another one)
  • You haven't defined it in the config schema, though it's extracted (if set)
  • But given the above and the fact that it's not used in selection, I am agnostic to including it in the config schema.
  • I would have said "include it in the schema" if we were using/displaying the info anywhere. But we aren't.

Then test.

Testing isn't hard. Once you test and see no strange behavior / warnings, I can merge it without looking at it further (just in this specific issue's case).

Promoting this to 'beta blocker', because this requires an interface break. (Specifically: widening some return value of getRenderer() and possibly introducing another interface.)

Most important tests are added, I may add a few more tomorrow.

Thanks. While I was testing, I encountered some PHP warnings which I fixed - along with tinkering with some strings.

Merged! (I don't know what that "18 commits" message is about, I only added a single commit and did not force-push. The only thing that happened is the "merge" button did not work the first time, because I had not approved the MR yet.)

Merged, thanks!

Two things:

1. My previous comment was dumb. In the "else", there IS no CE display, so you also cannot add it.

2. This is incomplete.

• Suppose you build a list of stuff in "teaser" view mode -- and the "teaser" CE display is not enabled, so it's using the default CE display.
• If this default CE display is edited, then caches for this list should be flushed. OK.
• If the "teaser" CE display is created/saved, then caches for this list should also be flushed, because they should be rebuilt with the new display. This did not happen yet.

So, cache tags should be added for both cases.

I figured it would be better to do this instead of setting needs work. Please review current status.

Great, thanks for testing / fixing!

I'm also adding it inside the "else" of buildEntityContent(), when the CE display is disabled. Because when it is enabled again, the same cache tags should be cleared.

But I can do that at the same time as a quick test.

roderik → created an issue.

roderik → created an issue.

Dumping draft comments I jotted down somewhere, to check at the same time. Not fixing for understandability - I will likely be the one to look at this anyway.

- take CoreFieldCeFieldFormatter
  - check buildConfigurationForm() & settingsForm().
  - what is what?						<-- settingsForm() is never used
  - when you understand that: fix the fact that CustomElementsFieldFormatterBase::settingsForm() / settingsSummary() have no definition in an interface AND STILL have @inheritdoc
    - then document that, in settingsform() (????), you should not use 'field_definition', 'view_mode', 'name', 'is_slot' -- because they will not be used.

roderik → created an issue.

What is mentioned in the previous comment, is done. Except that 'hidden' is still output as "null", for reasons (that I might look up again if I feel like it)....

I'm leaving this open for a doublecheck:
- After 📌 Simplify entity-ce-display form and AJAX logic Active is done: can we / have we got rid of the 'region' value?

Discussed Wed 14 August with @fago, who said there's nothing against just dropping the explicit dependency on custom_elements. We depend on it indirectly through lupus_ce_renderer.

If I get to be the person to make the next lupus_decoupled release, I'll merge this. (It's not on my direct timeline ...yet.)

roderik → created an issue.

See spec in 📌 Consistently prefix components with drupal- Active

Tested this, it mostly works good, except when you press save immediately without the AJAX routien to run.

Exactly. I took a little look, but did not find a way to remove the AJAX behavior on the name field immediately, because it's too tied in with the Core display form.

And since we are going to untie that relationship anyway, in 📌 Simplify entity-ce-display form and AJAX logic Active ... I didn't waste time on this and just put in the warning (because more than one 'tester' was bitten by this behavior.)

I think we need a follow-up for this todo introduced here

(this and several other comments above.)

I don't understand. The followup is #3446485, which is mentioned in the todos. I don't think we need separate followups.

• This (config change) was a big ticket, especially because its structure is antithetical to the current form (because the parent-form code that is hardwired to "a row is always a field", which this code is explicitly moving away from)
• #3446485 (redoing the form and untying the parent form's "a row is a field" logic) is going to be a big ticket. I suspect this is hard to split out in several MRs. Changing the form is just... big. (But I'll keep in mind, whether I can stop at a certain point, after I untied the parent-form-class. If I can, I will create a sub-MR or subtask for #3446485.)
• So these two tasks are linked together. But it would have been foolish to do both at the same time in a single PR.
• Therefore we needed a temporary situation where the new config format is clunkily tied into the old incompatible form, with temporary 'glue' code, in several places.
• For this I added loads of @todos as reference / justification / reminder to undo all of these in #3446485. All that code is going to be re-simplified or deleted or (in the case of getRenderer()) un-done when it's not necessary anymore.

Yes, #3446485 will be quite big, and I am going from the assumption it cannot be split up (though I'll re-check that assumption when coding). The 'good news' is: much of it is going to consist of re-simplifying things.

Re. ✨ Allow configuration of static props Active comments

I don't think we should todos for future plans in code. Let's keep code in slim and better remove it.

I will keep that in mind. When I was looking at implementing the structure change (and trying to do all I could to prevent a future compatibility break / internal-project-update-path), I felt that my changes were so invasive that I needed to comment this.

Indeed it would have been better to clean up my code after my ideas were 'settled' and tested, and move these to comments on the MR.

I'll merge your MR (after removing double "are" in "Since v3 processors on entity-level are used only when ..."

I was a bit unsure if @fago needed to see this. Now he has.

Merging.

roderik → created an issue.