🇦🇺Australia @larowlan

Just one comment on the MR, fine to self RTBC after that I think

Thanks! Looks great so far, but needs an upgrade path (post_update function?) for existing Component config entities (anywhere else?) unless those are automatically regenerated with this new expression on cache clear.

They will autogenerate but I _think_ that will create a new component version, so any content pointing at the old version will likely get double escaped. We might want to do an update hook to update the version of any content pointing to that version.

Credited @shitalb who also asked about this when the issue was private and was added there.

Moved the private MR to this issue

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

larowlan → created an issue.

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

@heyyo I've added some changes and additional tests to a new MR here.

The test shows we can recover from:

* Missing component
* New props (including new required props)
* Old props still present

Can you give it a test?

Thanks in advance!

So far so good w.r.t not needing an upgrade path.

larowlan → changed the visibility of the branch 3532514-robust-generated-props-form to hidden.

Pushed two extra commits to a new branch in 3470422-be-nice-for-sdc-developers-larowlan

I'm planning to use that approach in 🐛 Deleting or renaming SDC prop will break xb pages using old version of the component Active

Also includes tests (that will fail with the current code in the branch for this issue) for when the missing component has slots and there are child components in those slots.

To resolve that I was forced to go with the approach in 3470422-be-nice-for-sdc-developers-larowlan, which is why I think we need that here. It is a best-effort attempt but without the update path we'll need for 🐛 Deleting or renaming SDC prop will break xb pages using old version of the component Active

Rather than rely on locale module's parsing of JS, we could also attempt .po extraction with a Vite plugin - the moduleParsed hook in rollup (which Vite uses for building for production) should afford us access to the AST where we could find calls to Drupal.t() in a cleaner fashion that the regex used by _locale_parse_js_file which has had issues in the past

Pushed the tests per #30 - I don't think they make sense for JSComponents or Personalization so marked them as skipped. Went with a simpler approach than renaming because I think we'll need to alter definitions further in 🐛 Deleting or renaming SDC prop will break xb pages using old version of the component Active and this plumbing lets us do that.

Fixed phpstan while I was at it.

Removing myself from the assigned field because with the basis of these tests in place I can expand on other cases in 🐛 Deleting or renaming SDC prop will break xb pages using old version of the component Active , setting it back to @penyaskito

assuming @penyaskito is asleep I'll push a rough skeleton of the test in #30 so I can expand on it in 🐛 Deleting or renaming SDC prop will break xb pages using old version of the component Active

Working on this on top of 💬 ComponentNotFoundException: Unable to find component Active

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

Typehints?

https://www.drupal.org/node/3509577 → is relevant here

Would the primary use case for this be in controllers and form builders?

In which case I think a trait and adding that trait to FormBase and ControllerBase (that already have the request) feels reasonable.

The alternative is we add a new argument resolver to http_kernel.controller.argument_resolver and a HtmxHelper value object that can resolve from the current request.

That would allow people to typehint their controllers with `HtmxHelper $helper` and it would magically resolve like you can do for Request.

@ivnish thanks for updating this.

I'm not comfortable merging it without a review from someone who isn't me

I wrote the original patch and there is a security risk here - I'd like someone to validate my 'this is safe because' before I merge it - because if it's not we're in SQL injection land.

Triggering an error now, and then returning FALSE without calling the access callback in D12 seems like the safest approach.

+1

Mostly having everything in the one place for editors and not having to explain a new concept/disconnect to them between Page and Node

So traditionally we have one content type that is a 'landing page' that has no template and is a free-form starting point using components.
I think what you're saying is we're forced to use Page for that.
Is there scope for a content-type (node type) that isn't using a template but has its own Canvas field?

I think this is misunderstanding what the purpose of this issue was due to an out of date issue summary. The purpose here wasn't to fix a theming mismatch, though that's how this issue was initially opened as, it's that we're not wanting to support/maintain the concept of an individual node using Canvas other than via an exposed slot in the content template.

Ah that wasn't clear from the issue summary - thanks for the additional context.

You mean between now and when exposed slots are implemented?

Yes. I was under the impression that exposed slots were post 1.0 - is that not the case anymore?

Note the theme template in #35 could live in Canvas module with a hook_theme for that suggestion that used 'base hook' => 'field'

Coming in late here, but this severely hamstrings early adopters and for little gain outside of those using Olivero theme.

Work we had in progress now needs to patch or decorate to reverse this change.

We had already addressed these 'nuances' with a separate approach as follows that I think is more robust.

1) New template field--component-tree.html.twig with zero wrappers

{% for item in items %}
  {{ item.content }}
{% endfor %}

This does away with the > selector issues

2) Simple hook to clobber out display fields that aren't controlled by manage display

<?php

declare(strict_types=1);

namespace Drupal\some_module\Hook;

use Drupal\Core\Entity\EntityTypeInterface;
use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\Hook\Attribute\Hook;

/**
 * Defines hooks for node fields.
 */
final class NodeFieldHooks {

  #[Hook('entity_base_field_info_alter')]
  public function entityBaseFieldInfoAlter(array &$fields, EntityTypeInterface $entity_type): void {
    $hide = ['created', 'uid'];
    if ($entity_type->id() === 'node') {
      foreach ($hide as $field) {
        if (\array_key_exists($field, $fields)) {
          \assert($fields[$field] instanceof BaseFieldDefinition);
          $fields[$field]->setDisplayOptions('view', ['region' => 'hidden']);
        }
      }
    }
  }

}

For us this made the display in the preview match the saved node output without hamstringing the ability to use Canvas to make node based landing pages.

Testing this against the body field for a markup input, I get a 500 error 'Missing host entity'

One thing I noticed is this doesn't work for the media widget

Adding related 📌 [PP-1] Refactor GeneratedFieldExplicitInputUxComponentSourceBase and FieldForComponentSuggester to need only SDC's ComponentMetadata, not SDC plugin instances Postponed

The number of e2e fails indicate this needs work

Found something spurious that might explain the transient nature of this - possible race condition

Added 🐛 Editing support for type: array Active

larowlan → created an issue.

FYI this no longer works anymore.
There was no e2e tests added here and in manual testing:

* sparkline never shows the edit form, it just spins
* image gallery will render with the 3 example values, but as soon as you edit it, it will only ever show the first item selected (even if there are multiple images selected)

Testing with a simple multi-value string shows the same result, only ever the first value is saved.

Semantically title is the same as aria-label

Committed to 11.x

Confirmed the existing change record → has been updated for the new options.

Nice one folks.

I think other user groups have their own project
Eg Melbourne Meetup has a project and uses issues there to assign credit for attendees

Committed to 11.x and published the change record.

Nice work

Wrong issue queue?

Left a review on the MR

https://www.drupal.org/project/s3client/releases/1.2.0 →

Thanks, I'll cut a 1.2.0 release and add the above to the docs page

This one needs a reroll now after 📌 Support dynamic forms using HTMX Active

Fine to self RTBC

✨ Add a tree editor for the whole hierarchy Needs review

Seems fine to me, can we have a test attribute in the existing tests that uses this feature and some asserts that it works?

Hey @joao.ramos.costa - I'd like to get a green pipeline first - https://git.drupalcode.org/project/component_blocks/-/jobs/1779488

will see if I can make that happen here

https://www.drupal.org/project/layout_library/releases/8.x-1.0-beta6 →

Tagging 8.x-1.0-beta6, thanks!

Committed to 11.x

Published the change record.

Thanks folks.

pushed a start on a test but playwright isn't playing nicely locally

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

Indeed is fixed in HEAD

Thanks for the kind words!

Committed to 11.x

Left the change record as a draft until 📌 Support dynamic forms using HTMX Active is in

📌 DX object to collect and manage HTMX behaviors Active is in so this can be re-rolled and the todo resolved.

Cutting a new release

Couple of comments on the MR, great work!

Just one question on the MR

Confirmed that https://www.drupal.org/node/3539472 → has notes about headers per request in #47

Left a minor question about test coverage on the MR

Looks good to me, nice work

Left a comment on the MR

In the original issue @tim.plunkett expressed similar concerns about a fixed list of plugin IDs. I share that concern and have proposed an API addition to resolve that.

I don't see any tests that call ::validate and ensure the constraint validator works

Similar problem space 🐛 100% height on body or html element breaks viewport height calculation Active

larowlan → created an issue.

larowlan → created an issue.

Rebased this off 1.x and added an 'experience_builder.entity' in gathered contexts if it exists.

We'll likely need /xb/api/v0/config/component to also support entity type and ID so we can populate contexts to dynamically filter the available components.

E.g. we won't have field blocks from Page available on Nodes.

This definitely blocks any sort of LB update path.
Unfortunately without this it also means we (PNX) won't be able to start using this on client sites.

I think this is probably the next piece in the puzzle after 📌 Add support for layout plugins Active and 📌 Make block.settings.inline_block:* fully validatable Active in a path towards an LB in place update

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

Left some questions on the MR

Rebasing off 1.x

Updated the CR to make it clear that password.options is optional.
Added docs to core.services.yml and default.services.yml

Thanks, creating a new release

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

https://www.drupal.org/project/php_password/releases/3.0.0 → has been released with the forward compat layer

Moved the requirements checking logic to a new private method which enabled me to reduce the cyclic complexity/nesting.

As this was just moving things around I think I'm still ok to mark this as RTBC. So doing so.

I will create a 3.0.0 release
Thanks all

I updated the change record to add an example of setting the parameters via a services.yml file
Fixed a couple of typos in the MR.
Going to try to simplify the hook_requirements a bit

Seems reasonable to me.
Can you provide a snippet of docs for the additional settings that I can add to the project page on merge?

There is already hook_mail_alter in core, that should suffice here
Thanks for working on a patch

nod_ → credited larowlan → .

wim leers → credited larowlan → .

nod_ → credited larowlan → .

Added follow ups
📌 [PP-1] Try to work out a way to update the preview on undo without a POST Postponed
📌 [PP-1] Try to rewrite undo/redo history for model changes made by other users Postponed
📌 [PP-1] Poll for other users changes to layout/model/preview using pending changes Postponed
📌 [PP-1] Editing a prop on a deleted component should reinstate it Postponed
📌 [PP-1] Bubble rejected moves to the user Postponed
📌 [PP-1] Component input updates should only change one prop Postponed

Should be green now

larowlan → created an issue.

larowlan → created an issue.

larowlan → created an issue.