- Issue created by @arthur_lorenz
- 🇦🇹Austria fago Vienna
I added the MR. The changed logic is nicely covered by the pre-existing test-case, so there is no need to create a dedicated one for this change. However, I noted that we lack coverage for testing that the generated CE for the layout-builder itself works properly, thus opened 📌 Better test-coverage for layout builder support Active for it. Fortunately, there was no need to touch this part here, so it's not a problem to continue without it atm.
The change implements the configured behavior without introducing a new option. Generally, there is no need for one, since all fields can be removed from the configuration to get to the same result. Because of that, I prefer to not introduce another option to avoid the convolution of UI/config with many options.
That said, this is a change that potentially changes the API output for existing sites, i.e. if someone left the fields in the configuration, they are going to be output now. Thus, I'll create a change record. we are going to tag 3.1 anyway, so it's acceptable to have some larger change in there also.
- 🇸🇮Slovenia useernamee Ljubljana
PR looks good to me.
For BC we could add an additional checkbox to add fields to CE display. A config option that is only visible if layout builder is checked and disabled by default.
- 🇦🇹Austria fago Vienna
Thanks for reviewing!
>One idea that I had to have more backward compatibility is to add additional checkbox to add fields to CE display. A config option that is only visible if layout builder is checked and disabled by default.
True. Generally, it seems a bit duplicating since you could simply remove all fields. So considering new sites, the checkbox seems unnecessary duplication and it would be better to not have the checkbox. However, I agree that it would be nice for the upgrade path.
Another idea: Write an update function that removes fields for displays which have layout builder enabled. That way we do not clutter the UI and have a save update. I'll make a follow-up to clarify details!
Automatically closed - issue fixed for 2 weeks with no activity.