🇺🇦Ukraine @mostepaniukvm

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

I encountered the same problem, and the worst part is that it makes the _weight field visible and allows modification.
From my understanding, this issue is related to the changes in 📌 Allow for deletion of a single value of a multiple value field Fixed , particularly this commit.
Changes in the PR seems resolves this specific issue for me but still needs more testing.

diff link to unmerged related issue
https://git.drupalcode.org/issue/custom_elements-3362625/-/compare/33596...

mostepaniukvm → created an issue.

mostepaniukvm → created an issue.

mostepaniukvm → created an issue.

Enhanced description and added ~200 chars length summary.

mostepaniukvm → created an issue.

Yes, it's client's project was using tons of module_load_include()
I was using latest phpstan-drupal 1.2.0
I agree that it sounds like phpstan-drupal problem and not upgrade_status.
Will test again, describe steps and report issue to phpstan-drupal

+1 RTBC

I've faced similar issues.
Project widely uses module_load_include()
I replaced it with \Drupal::moduleHandler()->loadInclude() and it solved most of the problems.

However, when dealing with multiple nested includes, PHPStan throws an exception: "\Drupal::$container is not initialized yet. \Drupal::setContainer() must be called with a real container."

As result, in "LOCAL SCAN RESULT" column of upgrade-status report I see reported problem with message: "A file could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude"

Works for me,
+1 RTBC

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

DamienMcKenna → credited mostepaniukvm → .

> I see, so let's keep it. should we just use "label" instead of "name" or just keep it as null value?
I'm a little uncertain if it's a good idea to use label. I quickly tried to change but faced a problem with default values. Just reverted commit. I know it's not clean to keep it but it can be just an iceberg of a bigger problem. I'd first work on other tickets needed for alpha and return to this problem later in a separate issue if it will still be a case.

> Let's better just delete it right now since it's less confusing. It's ok if there are troubles with new netity-types until it is fixed, it's a blocker anyway.
Ok, I hope we understood each other correctly. I deleted all previous code from DefaultContentEntityProcessor and forced processor to generate custom element only from entity_ce_display config entity. Please take a look at PR and if something I will revert/change the commit.

> As disucssed, this is not needed, so comment can be removed.
done

> operation-link should be "Manage custom element display" just as the tab, not 'title' => t('Manage CE display'),
Corrected

> This must use proper DI, constructor may not use \Drupal.
Was copy-pasted. Moved into a separate method.

> Comment why you are doing it, not what you are doing. We want all the fields.
Totally make sense. Corrected.

> as discussed, we should render via $entity_ce_display by default, else if not existing, we can render nothing / an empty CE.
ok, it is deleted

> It should always render using the field formatter. If not a slot, simply set the string value as attribute value - but the value should not be different just because the "is_slot" checkbox got checked!
Alright, as I get renderable array in $build and not a string I need to render it first. I made the change to do it but I see it weird that attribute values can contain other HTML tags. Maybe I understood you wrong. Please see a change in PR.

I've made a few changes accordingly to what we discussed at the last meeting.
- better explanation for ::isSingleFieldDisplay() method
- tried to remove label from components in entity config export but apparently it's a little more tricky. After deleting it from config schema I've got failed tests. I added few workarounds to force clean it up but I always run into new bugs. The reason is that I still have EntityCeDisplay::displayContext = "view" Probably we can consider changing it but still not sure it is a good idea until we still using entity_view_mode entities and not completely new.
- made changes to DefaultContentEntityProcessor to use entity_ce_display configurations by default with the fallback to default view mode if the given doesn't exist. I kept the current logic still as fallback but after 📌 Auto-generate default CE display config per entity type Active it can be deleted.

@fago
Despite I merged the PR still assigning this to you for review. As already discussed, I'd recommend looking at 3.x -> 2.x diff as it will give you a better overview. And you can write all remarks here or in the follow-up issue.

Added meta issue for custom_elements first alpha on 3.x branch
🌱 [META] custom_elements_ui: 3.x alpha release Active

mostepaniukvm → created an issue.

mostepaniukvm → created an issue.

mostepaniukvm → created an issue.

mostepaniukvm → created an issue.

mostepaniukvm → created an issue.

We had a productive discussion with @fago

As part of this issue, I need to do a few corrections:

• remove <code>label</code> from components in entity config export;
• comment and explain better isSingleFieldDisplay() method;
• clean-up code about checkbox in entity_view_display, probably will go in follow-up;

Next issues that must be done for alpha (to create meta issue and sub-issues):

• we need to auto-generate default CE display configuration on module install, in hook_update(), and on entity type creation;
• CustomElementFormatter" plugin:
  • introduce new plugin type in addition to formatters;
  • make plugin type not configurable;
  • have attribute to decide whether it uses field formatters or not; usesFieldFormatters
  • in UI display only select with available options;
  • default plugins in a module are:
    • auto
    • raw
    • formatted
  • show in new column next to "isSlot", dropdown of CE-formatters. Try to make field-formatters drop-down disabled based upon "usesFieldFormatters" value of the selected plugin
  • plugin adds data to custom-element object given the configruation (name)
  • it thus can use string|MarkupInterface for adding a slot, or a single CE or an array of CE objects
    • raw plugin with toArray() might need something like var_export() to map to String
    • if not a slot, passing an array is fine

More issues to create:

• clean-up on bundle removal or module uninstall;
• the follow-up to check/extend config schema;
• move checkbox in entity_view_display to follow-up issue;

I finished with the initial implementation. I can't say that implementation is complete but it's ready for testing basic functionality.
Before continuing I'd like to get this reviewed and tested to get familiar with it before further discussion.

Open questions:
1. For field output configuration it uses formattes plugins and I added 2 dummy formatters for "raw" and "auto".
We need to think what will be best final solution:
- go with formatters;
- introduce new plugin type (with the possibility to reuse formatters as derivatives or maybe some tricky plugin discovery)

2. "Is slot" implementation.
- will we have some conditional formatters available depending on if the field is slot or not?
- consider creating two regions for attributes and slots instead of one

3. Discuss rendering logic:
- how entity_ce_display work together with core's entity_view_display?
- how entity_ce_display work together with existing processors? will it replace processors at some point?

4. Test coverage

5. helper functionality:
- delete entity_ce_display entities when somebody deletes bundle
- delete entity_ce_display entities on hook uninstall
- auto-generate/sync entity_ce_display from entity_view_display

6. ExtraFields support is not tested. is needed?

for both 2.x and 3.x branches configured next tests:
- PHP 8.2 & MySQL 8, D10 - on commit
- PHP 8.1 & MariaDB 10.3.22, D9.5 - weekly to see if no regression on Drupal 9

Merged to 2.x and 3.x branches and configured automated tests running

Did a few more corrections and now it works. Needs review

It will be better to finish with this issue and continue working on this newly created issue:
https://www.drupal.org/project/custom_elements/issues/3359601 📌 custom_elements_ui: re-implement with new concept Needs review

@fago
Changes in this issue I still consider as an improvement to the existing implementation that we have in 3.x branch. It contains multiple minor improvements and fixes a variety of regressions.
We had a chance to test it briefly during our last discussion thus I set it as RTBC and merge PR to close this issue and continue in a separate follow-up issue.

mostepaniukvm → created an issue.

Thanks for the contribution, I did a brief review of the related ticket first but closed it as duplicate when realized that we already have this.
Bringing the same comment here.

I checked the hook in PR and realized that the text seems just copied from README.md
Maybe it's already better than nothing but I still think it's not complete and maybe we can improve it to make it more useful.
Main concerns I see:
- avoid documentation duplication;
- make it really valuable and useful for site builders and developers;

I checked and see some documentation on drupal.org that I'd like to check first
https://www.drupal.org/node/632280#s-help-text-on-administration-pages →
https://www.drupal.org/node/632280#s-help-page →
https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... →

Additionally, I will bring up this question at some future meeting

Ok, now I realized that this issue is a duplicate of 📌 Implement hook_help() Closed: won't fix

I close it as a duplicate and we need to finish it there.

Thank you for your contribution! I think it can be useful to add hook_help() implementation.

I checked the hook in PR and realized that the content seems just a few first sentences copied from README.md
Maybe it's already better than nothing.

I checked and see some documentation on drupal.org that I'd like to check first
https://www.drupal.org/node/632280#s-help-text-on-administration-pages →
https://www.drupal.org/node/632280#s-help-page →
https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... →

I still think it's not complete and maybe we can improve it to make it really valuable.
Main concerns I see:
- avoid documentation duplication;
- make it really valuable and useful;

I will bring up this question at some of the next meetings.
Set status to Needs Review.

It looks good! Merged changes to both 2.x and 3.x branches.

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

We had a productive discussion with @fago and discussed multiple problems. Because of multiple limitations of the current implementation, we decided to make some major changes to make implementation cleaner and easier maintainable in the future.
We will do implementation in a similar way as implemented in core for entity_view_display and entity_form_display ("Manage display" and "Manage form display" tabs)
- config: core.entity_ce_display.ENTTIY.BUNDLE.yml
- toggle in Manage-display just controls whether we take over default entity rendering
- Manage-custom-elements config always applies when using the CE-generator for the entity and view-mode

I did implement multiple changes/fixes/improvements. Now custom_elements_ui will be more stable and probably enough to tag the first alpha release on 3.x branch.

During implementation, I did a summary of changes that will leave here for further discussion. Additionally, there are multiple @todo leftovers in the PR that can duplicate these notes (but better have twice than forget).

1. Checked briefly but I don't see a straightforward way to replace existing EntityViewDisplayForm routes with custom elements UI form.
To implement it we probably need to alter route and use our own form that extends the existing EntityViewDisplayEditForm
Theoretically, it should work but I am afraid to not break the existing View Display Form in case when custom elements UI is not activated for some display modes. Additionally, it will require to extend @internal class EntityViewDisplayEditForm and change form_id. In every method we will have checking like if (!$this->isCeEnabled()) { return parent::method()} As alternative we can move all implementation into hook_form_alter but I don't find it good;
As an easy solution I added redirect to CE form when user activates it for the first time.

2. The default value for custom element name is set to [entity_type_id]-[bundle]-[display-mode]
Or maybe keep it just node as processors do? To work the same after default activation
For fields' default names we just trim field_ prefix if present

3. Fix bug when you make field disabled but don't think it is good for a permanent solution.
The problem is that draggable javascript stopped working when I dropped field formatters.
As a quick workaround, I just make them not visible to keep js handle rows properly.

4. Added a fix to unset configuration if you deactivate custom_elements_ui.
I think would be nice to implement hook_uninstall too.

5. added changes to show all fields regardless if display is configurable

6. The infinite loop problem is little more tricky and needs extra discussion. Added temporary workarounds to get it work.

7. Changed output if "auto" formatter is selected. Probably it related to #3323558: Make configurable Auto option build Custom element same as existing processors do →

@fago Please review PR and you can already test it but let's additionally discuss it

mostepaniukvm → created an issue.

mostepaniukvm → created an issue.

Totally makes sense, thanks!

Merged, thanks everybody!

@fago
Thanks for your feedback here. I also see it about the same way.
I would implement a Generic processor for entity_reference_revisions field items but not for entity_reference
I would do similar to existing formatter defaults in field types annotations:
- entity_reference field items rendered as links by DefaultFieldItemProcessor similar to core's definition
https://git.drupalcode.org/project/drupal/-/blob/10.1.x/core/lib/Drupal/...
- entity_reference_revisions render entities by default
https://git.drupalcode.org/project/entity_reference_revisions/-/blob/8.x...

@Leo Pitt Patch looks good now the only thing that I don't sure still is the class name.
To be consistent we would need to change class name. Even though it is quite long I would call it EntityReferenceRevisionsFieldItemProcessor. @fago what do you think?

changes definitely will go to 3.x branch but what do we need to do with the existing ParagraphFieldItemProcessor in 3.x?
I think it's ok to just remove it, his job will be done by newly introduced EntityReferenceRevisionsFieldItemProcessor.

@fago you're right about configurable_custom_elements
It must definitely give more flexibility with any possible entity_reference widgets.

@fago Created a small PR, locally tested and seems to work well. I think we can merge and finish here?

@fago I tested and it works well. I left a comment in an old thread that I think is still actual. Could you confirm?
Because that comment is a little unrelated to this bug report issue I merge this PR and depending on your answer I can quickly add a follow-up commit or even a follow-up issue.

I just realized that this change is also not backward compatible. I will commit it only to 3.x branch and remove from 2.x

fago → credited mostepaniukvm → .

Merged into 3.x branch. Also synced 3.x with the latest changes in 8.x-2x

I checked and not sure if media_entity_instagram is needed at all. Tried to delete it but got other failures that need to be corrected in tests. It shouldn't be a big one. Meanwhile, I changed the configuration to trigger tests with 9.5

I tested the patch locally. Thanks for your contribution it's an indeed nice feature. It works well in general but unfortunately, it doesn't allow you to really switch to JSON content format. I investigated around and see that it's a little tricky and maybe even a challengeable as we first render view and later build custom_element from resulted HTML.
Maybe we need to research if there are any other possible solutions that allow keeping control of content format in one place in code and avoid major changes in architecture.

Additionally, I noticed that not only view rows were rendered into "rows" slot but also extra views wrapper tags. And as result we have outer rows slot and inner views-rows custom element. Maybe we can play with it to make the rendered content structure a little cleaner.
Example:

<view>
  <template #rows>
    <div class="views-element-container">
      <div class="view-id-test view-display-id-page_1">
        <div class="view-content">
          <views-rows>
            <template #row>
              <drupal-markup>
                <article role="article" class="node--view-mode-teaser">
                  ...
                </article>
              </drupal-markup>
              <drupal-markup>
                ...
              </drupal-markup>
              ...
            </template>
          </views-rows>
        </div>
      </div>
    </div>
  </template>
</view>