Live feed

โšก๏ธ Live updates new comments are added automatically.

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

๐Ÿ‡ฎ๐Ÿ‡นItaly mondrake ๐Ÿ‡ฎ๐Ÿ‡น

Updated IS.

๐Ÿ‡ฎ๐Ÿ‡ณIndia Tirupati_Singh

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!

๐Ÿ‡บ๐Ÿ‡ธUnited States hooroomoo

hooroomoo โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฉ๐Ÿ‡ชGermany donquixote

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.

๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States museumboy

We have done that with a custom module

๐Ÿ‡บ๐Ÿ‡ธUnited States m.stenta

Added a test for this.

๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States jnicola

I'll try and get that out here soon!

๐Ÿ‡ง๐Ÿ‡ทBrazil brunomolica

Have any of you could fix this issue?

๐Ÿ‡ณ๐Ÿ‡ฑNetherlands balintbrews Amsterdam, NL

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States m.stenta

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States m.stenta

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

๐Ÿ‡ซ๐Ÿ‡ทFrance pdureau Paris

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

๐Ÿ‡บ๐Ÿ‡ฆUkraine danmer

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

  • pdureau โ†’ committed fc10d3af on 1.x
    Issue #3505217 by grimreaper, pdureau, jmaxant: Support render arrays...

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

๐Ÿ‡ซ๐Ÿ‡ทFrance pdureau Paris

ok, i change a little thing and i merge

๐Ÿ‡บ๐Ÿ‡ธUnited States thejimbirch Cape Cod, Massachusetts

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

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

๐Ÿ‡ญ๐Ÿ‡บHungary lonalore Budapest, Hungary

Same issue here. MR fixed it. Thank you!

๐Ÿ‡ซ๐Ÿ‡ทFrance pdureau Paris

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

๐Ÿ‡บ๐Ÿ‡ธUnited States pookmish

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

  • m.stenta โ†’ committed 0caaf690 on 8.x-1.x
    Issue #3524480: "Date and time" datefield schema should use format "date...
๐Ÿ‡บ๐Ÿ‡ธUnited States swirt Florida

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

๐Ÿ‡บ๐Ÿ‡ธUnited States phenaproxima Massachusetts

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". ๐ŸŒณ

๐Ÿ‡ฎ๐Ÿ‡ฑIsrael jsacksick

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

๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

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.

๐Ÿ‡บ๐Ÿ‡ฆUkraine danmer

danmer โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

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

๐Ÿ‡บ๐Ÿ‡ธUnited States dww

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++

๐Ÿ‡บ๐Ÿ‡ธUnited States museumboy

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

๐Ÿ‡บ๐Ÿ‡ธUnited States drumm NY, US

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.

๐Ÿ‡บ๐Ÿ‡ธUnited States phenaproxima Massachusetts

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.

๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

@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 ! ๐Ÿ˜„

@catch in #28: many of those concerns have been discussed in ๐Ÿ“Œ [PoC] Introduce a `ContentTypeTemplate` config entity Active and in โ€ฆ calls ๐Ÿ˜ญ. But they definitely don't have full/final answers yet. The idea was to make things more concrete by incrementally implementing, and identify unknown unknowns ๐Ÿคž

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.

๐Ÿ‡ต๐Ÿ‡นPortugal claudiadesiderio

claudiadesiderio โ†’ changed the visibility of the branch 3524484-adding-calculations-for to active.

Production build 0.71.5 2024