Problem/Motivation

This module currently uses custom Javascript and AJAX markup to dynamically update dependent form input. With HTMX in core, we should evaluate the effort required to implement that instead.

Proposed resolution

Create an implementation that uses HTMX to achieve the same result.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mandclu created an issue. See original summary.

mandclu’s picture

Status: Active » Needs review
mandclu’s picture

Status: Needs review » Needs work

Feedback from Codex:

I’d mark the MR Needs work, mostly for access-control hardening. The HTMX direction is good, and the core API usage is broadly right.

What Looks Good

The overall replacement of the Redux-polling custom JS with Drupal core HTMX is a solid improvement. Htmx::applyTo() automatically attaches core/drupal.htmx, so removing canvas_field_component.libraries.yml and js/component-form-refresh.js makes sense. HTMX also includes the enclosing form values for non-GET requests, so the form-context POST strategy is reasonable.

MR pipeline is green: MR !3.

Main Concern

The new route is too permissive:

yaml
requirements:
  _permission: 'access content'

Because FormatterSectionForm reads arbitrary posted canvas_component_props and uses those to load field definitions / formatter settings, anyone with access content could POST entity type, bundle, field name, and formatter IDs to get form fragments back. That permission is often available to anonymous users. This endpoint should use the same access model as editing the Canvas template/component instance, or a custom access checker that validates the actual editable template/context. At minimum, it should not be access content.

Relevant code:
canvas_field_component.routing.yml:
src/Form/FormatterSectionForm.php:46-83

Secondary Concerns

The MR includes an unrelated rendering-path change: replacing FieldItemListInterface::view() with a cloned EntityViewDisplay path in buildFieldRenderArray(). That may be a good fix, especially for third-party formatter settings needing display context, but it is behaviorally broader than “replace custom form refresh with HTMX.” I’d either split it into a separate issue/MR or explicitly document why the HTMX change requires it.

There are no test changes in the MR. I’d want at least coverage that:

- the field select has the expected HTMX attributes;
- the formatter section POST returns only the formatter section;
- the route denies users who cannot edit/configure the Canvas template;
- formatter changes still preserve/update settings correctly after swap.

Suggested Maintainer Feedback

“Approach looks promising and the Drupal core HTMX usage is appropriate. Before merge, please tighten the HTMX route access so it matches Canvas template editing permissions rather than `access content`, and add coverage for the fragment endpoint. Also consider splitting the buildFieldRenderArray() rendering-context change into a separate issue unless it is required for this one.”

Sources checked: Drupal `Htmx::applyTo()` API, HTMX request parameter docs.

mandclu’s picture

Status: Needs work » Needs review

  • mandclu committed ea38f850 on 1.0.x
    feat: #3587445 Use HTMX for dynamic form updates
    
    By: mandclu
    
mandclu’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.