Live feed

My votes:

Will Huggins (zoocha-will)
Marine Gandy (Mupsi)

Automatically closed - issue fixed for 2 weeks with no activity.

Updated IS.

Hi all, I've also attempted to replicate this issue while working with the components. For example, with the "Carousel" component, when I passed the following values:
show_carousel_control: true,
show_carousel_indicators: false,
show_carousel_caption: false,
the carousel still rendered indicators and captions, even though false was explicitly set for those props. This behavior was consistent across other components as well, whenever the default('true') filter was used. As @danchadwick mentioned in comment #6, the default filter was causing this unexpected behavior.

I applied the provided MR as a patch, and it applied cleanly without any issues. After the patch was applied, I observed the components working as expected. Specifically, replacing the default('true') filter with the null coalescing operator (??) resolved the problem. Now, when passing show_carousel_control: true, show_carousel_indicators: false, and show_carousel_caption: false, the carousel no longer renders the indicators or captions, as expected.

I've attached screenshots demonstrating the change and its effect on the component for reference. I am moving the issue status to RTBC, as the changes are now working as intended across the components.

Thanks!

carlos romero → opened merge request !30

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

Cool. Feedback applied.

Why would we want to remove method from extensions but not the root hook attribute.

Simply because we can remove it from subclasses, but removing from the main class is a BC break.
(unless we do some ugly tricks)

Honestly I think we want to keep them in sync, either keep the parameter on all our remove it from all.

I would agree that keeping them in sync could be desirable.

However, if we keep the $method parameter on all the subclasses, later we will have to remove it from all of them.
Whichever solution we find to clean up the main Hook class, will then have to be duplicated to all the subclasses.

My argument here is to remove it from the subclasses while we still have the chance.
Then for the main Hook class we can remove it when we are allowed to break things.

carlos romero → created an issue.

The immediate concern I have with not having that is: data storage of XB would then still change at a later time.

It shouldn't change.

If ✨ Add way to "intern" large field item values to reduce database size by 10x to 100x for sites with many entity revisions and/or languages Needs review follows the current proposed solution, the 'interning' would be a configurable (per field instance), internal detail of the sql storage. It would be transparent to XB and the field definition etc.

It may not be straightforward for existing sites to change that on a field that already exists (would probably need a custom update path, maybe to a new field name), but this is not the same as XB itself having to change its data model and provide an upgrade path.

If #2770417: Revision garbage collection and/or compression → happens, there is no change to the data model at all, just occasional pruning of older revisions. The two issues also aren't mutually exclusive although if we do one of them, the other becomes lower priority.

We have done that with a custom module

Added a test for this.

herved → opened merge request !14

solideogloria → created an issue.

Found the revision pruning issue, linking it here.

Automatically closed - issue fixed for 2 weeks with no activity.

Automatically closed - issue fixed for 2 weeks with no activity.

jimafisk → created an issue.

I'll try and get that out here soon!

Automatically closed - issue fixed for 2 weeks with no activity.

Have any of you could fix this issue?

m.stenta → opened merge request !18

Yes, that issue didn't affect what's being reported here. So this issue is still relevant.

How relevant? Our starter code component code explicitly says "Do not include @import 'tailwindcss'", and it also demonstrates how Tailwind CSS just works without that, so I recommend we change the priority to minor. 🙂

Lauri, please bump it back in case you disagree.

For some reason, the D11 changes were only made on 1.x.

https://git.drupalcode.org/project/restrict_route_by_ip/-/commit/059cc32...

These changes need to be made on 2.x as well.

Thanks @stefanlehmann - I'm going to fix this in 🐛 Decimal and float fields have a null schema Needs work , which was originally focused on float field types, but the fix is the same for decimal so I'll do both at once.

This also applies to decimal fields. Updating title and description accordingly.

Also, marking this as "Major" because including either of these field types in an entity bundle crashes the schema for that bundle with the following error. This may have changed recently due to upstream changes in Symfony, because I don't think I experienced this originally, but it was also reported in 🐛 Decimal field causes error message on custom entity Active .

The website encountered an unexpected error. Try again later.

TypeError: Symfony\Component\Serializer\Serializer::normalize(): Return value must be of type ArrayObject|array|string|int|float|bool|null, stdClass returned in Symfony\Component\Serializer\Serializer->normalize() (line 161 of /var/lib/tugboat/stm/vendor/symfony/serializer/Serializer.php).

Drupal\jsonapi_schema\Normalizer\ListDataDefinitionNormalizer->normalize(Object, 'schema_json', Array) (Line: 32)
Drupal\jsonapi_schema\Normalizer\FieldDefinitionNormalizer->normalize(Object, 'schema_json', Array) (Line: 161)
Symfony\Component\Serializer\Serializer->normalize(Object, 'schema_json', Array) (Line: 232)
Drupal\jsonapi_schema\Controller\JsonApiSchemaController->Drupal\jsonapi_schema\Controller\{closure}(Array, Object)
array_reduce(Array, Object, Array) (Line: 231)
Drupal\jsonapi_schema\Controller\JsonApiSchemaController->addFieldsSchema(Array, Object) (Line: 210)
Drupal\jsonapi_schema\Controller\JsonApiSchemaController->getResourceObjectSchema(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 741)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I've learned a lot about how this module works since I posted the patch in #4. The correct approach is to provide a new normalizer class that declares supported types of decimal and float. I will open a merge request with tests shortly...

It will be possible as a side effect of the new Content context and its related sources plugins for slots: ✨ Add content source Active

@solideogloria sure, I want to fix all PHP Sniffer issues, I'll fix the \Drupal calls as well, thanks

Is it possible to use service injection and avoid the \Drupal calls in the class?

ok, i change a little thing and i merge

This looks great. Moving to RTBC. I also added a change record.

I have updated the patch so it can fix with the last compatible version of D11.

Automatically closed - issue fixed for 2 weeks with no activity.

Same issue here. MR fixed it. Thank you!

Oops, it was not something to merge yet. Merge train cancelled.

@jnicola, The patch no longer applies after the merge of the meta tag change. Can you please move this change into a MR. It will be easier for review and any feedback that might be needed.

I like this idea. I've committed your patch.

genklegni

Automatically closed - issue fixed for 2 weeks with no activity.

I'm very tempted to make "bonsai trees" an official term in XB, so can you elaborate on why you used that term and your rationale?

As I understand it, a bonsai tree is, physiologically speaking, a regular tree. The only reason it doesn't reach full size is because it's planted in a small pot and its growth is there constrained.

The data in an exposed slot is very similar. It's a completely normal XB tree, just rooted in a small pot (a single slot) of a larger garden (the content template).

"Subtree" is also a reasonable way to describe it, but that's more technical and less pleasing than "bonsai tree". 🌳

@rhovland: Does the current patch allow for that? (from the MR)? I'm considering merging it as it looks ready.

danmer → opened merge request !61

I ran into this issue as well while trying to use the module with Single Content Sync → .

Is there any update of what tasks remain to complete this issue or feedback from the module maintainers on the right approach.

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

Rebased the MR.

Just caught up on #3519352 — high-level responses at #3519352-38: Content templates, part 3b: store exposed slot subtrees on individual entities → .

Makes me want to #22+++++++++++++ even more than I could've anticipated 😄 — as in: "do this first, but keep #3519352 in mind".

tyler.hastain → created an issue.

I’m still not convinced, but not worth it to keep arguing. I’m happy to stand aside and let the majority view prevail here. 😉 HookDocumentation it is.

I think the only other concerns in here are very minor docs nits that could be fixed any time, so let’s get this in before beta gets tagged and we can’t fix any of it.

RTBC++

After I applied this patch and changed the format. I no longer got a PHP error, but the field value isn't saving.

You can see the project is migrated at https://www.drupal.org/jsonapi/node/project_module?filter[field_project_... →

How project browser is setting up filters for querying might be filtering out the project for some reason.

martonlaci → created an issue.

We shouldn't have the ID be generated by conditions ("if too long, do this"). We should always do the same kind of processing to it, so that it's as deterministic and consistent as possible.

@larowlan in #21:

one about an edge case of a slot named '0'

Another reason to do 📌 Constraint slot names allowed by XB in its component tree Active ! 😄

I'm glad to see @lauriii already said as much in #29 😊

WRT deleting component subtrees (for this issue: of unused slots' subtrees): ✨ [later phase] Provide API for finding and UI for surfacing dangling/dead component subtrees — aka garbage clean-up Postponed — but also of defunct components, which is necessary because of 🐛 Cannot delete JS components due to component depending on them Active .

@phenaproxima in #32:

the frontend has absolutely no concept of "this tree will fit into a slot".

1. That's intentional: docs/adr/0005-Keep-the-front-end-simple.md
2. That's for later: ✨ Component restrictions Active
3. Yes, the client side will need a new "node type" for its client-side data model — see 3.4 UI Data Model: communicating a `component tree` to the front end in docs/data-model.md — currently only 3 node types exist: component, slot and region. I do agree a new one will be needed for this. That's what 🌱 [META] Production-ready client-side data model + internal HTTP API Active exists for.

P.S.: I'm very tempted to make "bonsai trees" an official term in XB, so can you elaborate on why you used that term and your rationale? 😄

@catch in #33:

RE: information disclosure risks Yep, I raised exactly that concern. 💯

The reason we have no choice but to allow this is that we know from the XB team members who've worked on https://github.com/acquia/cohesion (aka "Acquia Site Studio") for years that site builders will accidentally delete a slot, confirm through all the warnings, and then later realize that actually they do want to undo it after all.

RE: Wow — glad you caught that! Had not crossed my mind yet!

@larowlan in #37: see the validation+config schema added in ✨ Content templates, part 2: store a component tree in the template, and allow individual entities to fill in specific slots in that tree Active — exposed slots are intentionally only allowed for the full/canonical view mode. Because otherwise you get different component trees per view mode, which would be an authoring/usability nightmare. I first identified this in #3511366-3: [META] Introduce a `ContentTypeTemplate` config entity + related infrastructure → , @lauriii responded at #3511366-7: [META] Introduce a `ContentTypeTemplate` config entity + related infrastructure → .2.

Apologies, @phenaproxima, for having sent you down this path 😬 Turns out that this "incremental step" was fraught with more challenges than I anticipated. I expected it to need refactoring later on for sure, but I tried to unblock you on next steps for content templates. Turns out that was not quite realistic.

Per discussion with @effulgentsia, we think this is best tabled until after 📌 [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed . @larowlan is actively working on spikes to help us make the best choice possible.

Agreed that an ADR is needed and appropriate, but during this read-through/catch-up, I did not end up with the impression there's only 2 possible choices as @lauriii indicates in #35 — I think there's many possible choices.

graber → opened merge request !9

graber → created an issue.

claudiadesiderio → changed the visibility of the branch 3524484-adding-calculations-for to active.