🇺🇸United States @apmsooner

Account created on 9 September 2009, almost 15 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States apmsooner

There is no UI to do it so an update hook in your own custom module is the way to go about it. A service is provided to assist you with this. Adding/Removing columns is documented here: https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... .

🇺🇸United States apmsooner

btw, i guess this is an oversight around Drupal 9 compatibility as those errors aren't happening in Drupal 10 because of this function:

/**
   * {@inheritdoc}
   */
  public function getSupportedTypes(?string $format): array {
    return [
      CustomFieldEntityReference::class => TRUE,
      CustomFieldImage::class => TRUE,
    ];
  }
🇺🇸United States apmsooner

Thanks for the patch. The EntityReferenceNormalizer however also supports CustomFieldImage so would either need to revise this to support both types or split that off into its own separate normalizer. Open to further review.

🇺🇸United States apmsooner

Checkbox is just a widget type that is already available for boolean data types. See https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... for the breakdown of whats available for each data type.

Checkboxes will not be supported as those values would require a separate table for storage. Custom fields are stored in a single table so therefore each column will only ever allow a single value.

🇺🇸United States apmsooner

@casey, my understanding is they are trying to eliminate perhaps the paragraphs model being in the mix of layout builder. I feel like we need to define some sort of desired json data structure for the layout object that consists of references to different things. I'm providing a proof of concept idea here sort of similar to the builder.io example file but specific to Drupal. What I "think" we need is:

  • Field(s) from current entity
  • Field(s) from some other entity via context? (Maybe a way to add additional contexts similar to how Panels worked...)
  • Field properties? - this is the part where field_union may come into play if you wanted to extract a specific sub-field value from the field.
  • Various entities in desired teaser mode
  • Views display with ability to pass arguments
  • Inline content - this is maybe similar to the old Panels "custom content panes"? But maybe something like storage entities could work here...
  • Sections - Sections have children that compose any of the above as well as other sections.
  • I think the context part of this is important for lookups, caching, etc... If a field gets deleted for example, a lookup by field properties should update this object. This could perhaps be flattened as an all encompassing array on the object to avoid deep nested queries within the json?

Here's a rough idea of what I was thinking. Maybe its not at all what is being considered but just thought id throw it out there as to me it makes sense to work out the data part first and build ux around that. I don't know that I would build in the props part as in css styles into this but rather maybe wrapping elements, classes, ids would be sufficient...

{
  "data": {
    "components": [
      {
        "type": "field",
        "context": {
          "type": "node",
          "bundle": "article",
          "name": "image",
          "id": 1,
          "view_mode": "teaser"
        }
      },
      {
        "type": "entity",
        "context": {
          "type": "block",
          "bundle": "basic",
          "id": 1,
          "view_mode": "default"
        }
      },
      {
        "type": "view",
        "context": {
          "id": "who_s_online",
          "display": "who_s_online_block",
          "args": null
        }
      },
      {
        "type": "section",
        "context": {
          "name": "section_1",
          "label": "Section 1"
        },
        "children": [
          {
            "type": "field",
            "context": {
              "type": "node",
              "bundle": "article",
              "name": "image",
              "id": 1,
              "view_mode": "full"
            }
          },
          {
            "type": "entity",
            "context": {
              "type": "block",
              "bundle": "basic",
              "id": 1,
              "view_mode": "teaser"
            }
          },
          {
            "type": "section",
            "context": {
              "name": "section_1_1",
              "label": "Section 1.1"
            },
            "children": [
              {
                "type": "view",
                "context": {
                  "id": "recent_articles",
                  "display": "recent_articles_block",
                  "args": [1]
                }
              },
              {
                "type": "view",
                "context": {
                  "id": "related_authors",
                  "display": "related_authors_block",
                  "args": [1, 2, 3]
                }
              }
            ]
          }
        ]
      }
    ]
  }
}
🇺🇸United States apmsooner

Ya know Panels + ctools was doing all this type of stuff being discussed over 10 years ago. The fields were "panes" either from current node context or some other other entity context. They could be rendered in whatever view mode needed. So a field_union type field can also have different rendering depending on view mode selected (ie; hide/show/format subfields differently).

Other stuff like views, blocks, custom content panes, etc... were all able to be pulled into a layout. There were contextual rules built for who sees what and various layouts that could be assigned to a page. I'm not saying it was perfect but I built alot of complex pages back in the day using panels when I was just a site builder. I assume there must have been valid reasons beyond my understanding to shift to layout builder. I only really build decoupled sites so I don't really have that much of a dog in this fight anyway but IMO layout builder was/is a considerable downgrade from what panels was already doing several years ago. I understand the panels configuration was still geared a bit more to site builders vs content authors but I personally think alot could have been (and maybe still be) adopted from that architecture that for whatever reason was just shelved. Perhaps I'm alone in thinking we're perhaps reinventing the wheel here maybe just a little bit?

🇺🇸United States apmsooner

Glad you got it working! As for...

It seems every time when you want to add/remove column you have to comment hooks out - enable module and later uncomment and run update.php.

No, this is incorrect. The issue was that you just didnt have your custom module enabled prior to the update hook being added. Keep it enabled and do NOT remove the existing update hook, just add new ones to it with the next sequence number. It's important to retain the correct sequence order. If you're last one is: custom_field_updates_update_10002, then your next one is custom_field_updates_update_10003.

🇺🇸United States apmsooner

Maybe try uninstalling the module, remove or comment out the update hooks from .install file, then enable the module. After module is enabled, add back in the update hooks and try running update.php again to see if they are picked up in pending updates. It sounds like maybe having them present before the module is installed may prevent them from being recognized.

🇺🇸United States apmsooner

Did you run update.php? These update hooks don't just run by enabling the module, you have to run update.php.

🇺🇸United States apmsooner

Closing this since no response back.

🇺🇸United States apmsooner

I'm not sure what you are doing with the 2 files. I would just put your update in your custom module .install file like example:

/**
 * Adds the "port" column to the "field_circuit" field storage.
 */
function custom_field_updates_update_10001(): void {
  /** @var \Drupal\custom_field\CustomFieldUpdateManagerInterface $update_manager */
  $update_manager = Drupal::service('custom_field.update_manager');
  $update_manager->addColumn('node', 'field_circuit', 'portx', 'string');
}

Not sure why you're wrapping the field name with html... thats definitely wrong. You shouldn't need any other options in this case either. Example above should just work for you. The sequencing number doesn't really matter but if you're on Drupal 10, I'd probably suggest 10001 as example. The updates just need to stay in order so if you add another one later... it would be custom_field_updates_update_10002()

🇺🇸United States apmsooner

Image fields have an extra wrapper added via ajax that prevents our classes from working correctly to get the styles. I don't unfortunately have a solution that gets around this and honestly think the inconsistencies in Drupal form widget wrappers due to use of ajax is a problematic issue to improve upon with theming.

🇺🇸United States apmsooner

Is this not something that can be handled in a custom module? I don't know if enough people otherwise are using both modules otherwise to warrant handling it here. Perhaps a patch for the support in single_content_sync module would be better if the maintainers are interested in it there.

🇺🇸United States apmsooner

The logic is the same and I cannot reproduce this issue. I'd suggest double checking if the referenced content actually exists and is published. Maybe create a separate simplified example and see if you get different results.

🇺🇸United States apmsooner

The ajax in widget settings is not working correctly with this new widget when changing reference method.

🇺🇸United States apmsooner

This is working now after a little refactoring (dropdown was empty for me). I will still want to probably create a base class to extend as described above just to simplify the code but its a good start and can get this wrapped up very soon.

🇺🇸United States apmsooner

Hey there, thanks for the patch! This has been on my radar too so happy to review this. I may need a day or 2 to get to testing it out but just wanted to let you know its not being ignored. I'm thinking now we might want to have an EntityReferenceBase class that both autocomplete and select extend since I'm noticing alot of the same code in both widgets.

🇺🇸United States apmsooner

@jurgenhaas - I suspect there would need to be specific plugin integration to make custom_field work with eca module. The custom_field module field types consist of its own plugin system so ECA wouldn't know (i assume) how to interact with for instance a custom_field entity reference field without accessing the the custom_field plugin for that column type. The column settings "type" value map up to the same named plugin ids: https://git.drupalcode.org/project/custom_field/-/tree/3.0.x/src/Plugin/....

Theres a helper function here that would return an instance of the custom_field type plugins based on field definition settings passed into it that may be helpful: https://git.drupalcode.org/project/custom_field/-/blob/3.0.x/src/Plugin/...

I have no knowledge of ECA unfortunately but if theres enough interest in making it work with custom_field, I'm open to helping out where I can. I suspect that further effort would live as a feature request in the ECA module at this point unless you have any insight of additional changes needed in custom_field module. I'll wait on additional feedback before I merge this patch in.

🇺🇸United States apmsooner

Here's a patch that can be tested.

🇺🇸United States apmsooner

Thanks for diligently pointing this out. I could have sworn i already had mainPropertyName() set in the field type but sure enough I'm not seeing it for some reason. Opening this back up and I'll provide a patch that can be tested.

🇺🇸United States apmsooner

There is no main property name with custom_field. It's not technically possible as the properties are dynamic so theres no way to know which property would be considered the "main one". If the ECA module is depending on that... I'd have to say its not gonna be compatible with custom_field and nothing I can really do about it from my end. jurgenhaas also uses the custom_field module so I'd suggest bouncing it back to him to see if there is anything differently that can be done for ECA.

🇺🇸United States apmsooner

Question @catch - technically could fields be completely detached from an entity? I don't even know if that's possible but if you could have named field instances with no reference to any particular entity then I "suppose" its something reusable that could be referenced anywhere. Maybe thats what this d7 module was trying to do: https://www.drupal.org/project/field_reference

🇺🇸United States apmsooner

That works if you have one image gallery with multiple images. What happens if you want two image galleries?

I would treat these as 2 separate field_union|custom_field types with likely the same setup. 2 image galleries attached to a single entity doesn't sound like a typical thing though.

🇺🇸United States apmsooner

Where field_union would also struggle a bit is if a component is something like an image gallery with an arbitrary number of images, in that case I think it would have to go back to a reference to an intermediate entity (block?) with an image gallery bundle, because each image is its own delta-within-a-delta then.

I think field_union is very similar to https://www.drupal.org/project/custom_field in this sense. A field_union field like any other field can be multi-valued. An image gallery would just be a multi-valued field_union consisting of an image field and whatever other sub-fields are set. The values would all be stored in a single field table with deltas just like any other field. Think multi-value address field as an example... same storage mechanism just flexible field types to choose the columns. Furthermore, I think we need to stray from thinking everything needs to be an entity. In the case of an image gallery for instance, the normal consensus is probably to build a component referencing a bunch of media items cause media has been so heavily promoted as the go to for images now. Image galleries are typically though specific to the content it's attached to thus the images will likely never be reused. Having additional entity wrappers in this case are unnecessary IMO and add to the bloat.

Paragraphs are getting unfairly thrown under the bus here I think. The entity_reference_revisions module is the underlying module which also supports blocks and other entity types as well. This is a content modeling decision ultimately around do i need per field instance revisions or can i live with a normal entity reference field that updates globally wherever its used. Paragraphs just currently has the best UX IMO for multi-value page components which is why I think most people use it. And compared to other CMS's... if you need per instance reference revisions, paragraphs is a unique offering from Drupal that those systems don't typically support. Now additional nested paragraphs within paragraphs is where it starts getting really ridiculous.

As for overall bloat and entity_reference_revisions aside, the field api IMO is the ultimate source of bloat. Storing a simple string value for instance as its own table has alot of extra unnecessary data on top of it. Any field just like entity_reference_revisions instances has a corresponding revision field that gets inserts on parent entity save whether something changed or not. Field api has always provided less experienced developers a way to model content types in a very flexible way so I'm not knocking it, but the amount of joins involved with querying all these field tables would never be an ideal way of storing data in enterprise level sites with alot of content. Savvy developers will make custom field types that optimize data storage but until now with custom_field and field_union coming soon, there has been no standardized way of doing this without alot of custom code.

Ultimately i'm happy to see this discussion happening because actual content vs. settings has typically been mixed together as fields on entities and this JSON-based data storage idea can be used in alot of areas aside from just layout builder.

🇺🇸United States apmsooner

Can you click the "set default value" widget to expose the default values form when you trigger the error to see if theres some sort of hidden validation going on there? I just added like 15 fields with no issues so something is a bit different with your settings perhaps.

🇺🇸United States apmsooner

Are there any errors in the logs or console errors related to ajax? This is odd...

🇺🇸United States apmsooner

Thanks, I will revise this and modify some stuff for uri and telephone fields to get tests to pass. The storage settings on those should actually not be changed so just need to modify some widget settings defaults.

🇺🇸United States apmsooner

I think this is likely related to the same issue in this bug: https://www.drupal.org/project/custom_field/issues/3442408 🐛 Max length storage/widget settings out of sync. Needs review so please apply that patch and report back if this issue is also resolved.

🇺🇸United States apmsooner

Please try the patch and let me know if this fixes the issue. FYI, the uri and telephone field types now have max_length setting exposed in storage settings to prevent further issues in case the existing field type changes.

🇺🇸United States apmsooner

I'm thinking I'll just make maxlength in the widget settings required and set the #value property as the same as the #default_value. The reason being is that the widget settings may have value of 255 but the column settings may have changed and have e.g. 100. The maxlength should basically always be set to no more than what the storage allows. I realize i also need to expose the storage max_length for uri and telephone as the type can change from string to uri for example and the max_length storage settings just get inherited without ability to override. I'll have a patch up shortly that can be tested for all this.

🇺🇸United States apmsooner

Thanks for the report. I am able to reproduce and will work on a solution for this.

🇺🇸United States apmsooner

Thanks for all the help with testing the patches. I will get a new release out soon!

🇺🇸United States apmsooner

The unit test passes now also so I know the node entity is working. I don't have a test built in for the commerce_store entity situation but it is definitely working for me locally.

🇺🇸United States apmsooner

This updated patch is working for me on both entity types for add/remove columns. I may revise some more to simplify it but its at least processing through and returning the correct results while preserving the existing data.

🇺🇸United States apmsooner

It appears the claro theme handles date field wrappers differently than gin theme (which extends claro). Gin theme works fine for flexbox although the help text position as you describe does not have affect. Date fields I think have some after build functions that modify the form output and each theme may handle this differently. I can see if theres a more generic solution to handle the classes but most people are probably using the gin theme so perhaps trying that might be an immediate solution for you.

🇺🇸United States apmsooner

Thanks for update. There seems to be some major inconsistencies with this revision table depending on entity type issue that I'm not yet sure how to completely account for. Back to the drawing board i guess for now until i can figure something out.

🇺🇸United States apmsooner

I don't really have time to provide a solution here as I'm not going to support the conditional fields module. That module is using the states api under the hood so all i can suggest is doing a hook_form_alter in your own custom module and using #states api for the visibility you need on these individual fields. Google search and/or AI may be way more help for you on this than me if you need full examples. Thanks.

🇺🇸United States apmsooner

This patch should resolve the issue. Please report back. FYI, The test is failing because a new flag that got introduced I believe is expected to be set when updating storage schema but its not yet clear to me how to set that in the service. Anyhow, its working consistently now on the commerce_store entity when i tested for both add/remove columns.

🇺🇸United States apmsooner

I dont quite understand why a revision table name exists when the the entity is not revisionable, but this addition allowed me to get the update to go through. I had to manually delete the column though in the db from the original fail as it still got saved. Just the revision table stuff is all kinds of odd so we can skip over it like this:

    $is_revisionable = $storage instanceof RevisionableInterface;
    foreach ($table_names as $table_name) {
      $field_exists = $schema->fieldExists($table_name, $column_name);
      $table_exists = $schema->tableExists($table_name);

      if ($table_name === $entity_type_id . '_revision__' . $field_name && !$is_revisionable) {
        continue;
      }

I'll have to test this more for a proper patch as there are also similar issues on the removeColumn().

🇺🇸United States apmsooner

Thanks for the report. Were you able to capture any specific errors? I can try to reproduce this and see if i can come up with a solution but this is a really odd issue that the table exists but it sorta doesn't really....

🇺🇸United States apmsooner

Update example.

🇺🇸United States apmsooner

Update example.

🇺🇸United States apmsooner

Remove deprecated notice.

🇺🇸United States apmsooner

Remove deprecated notice.

🇺🇸United States apmsooner

Add file/image types.

🇺🇸United States apmsooner

This is not a bug with the module. Ckeditor toolbar in Drupal core by default doesn't wrap toolbar buttons so it breaks the layout. The solution for this is to drag a wrapping button divider in the ckeditor configuration into the active toolbar as described here: https://www.drupal.org/project/drupal/issues/3328095#comment-15173888 🐛 CKEditor 5 toolbar overflow can become unusable in Off canvas Needs work

Production build 0.69.0 2024