Closes: #15165 - HTMX partial fieldset re-rendering for HTMXSelect forms#22345
Closes: #15165 - HTMX partial fieldset re-rendering for HTMXSelect forms#22345bctiemann wants to merge 10 commits into
Conversation
|
@claude /review |
|
Claude finished @bctiemann's task in 4m 13s —— View job PR Review: feat(ui): HTMX partial fieldset re-rendering for HTMXSelect formsTodo List
OverviewThis PR replaces full
The interaction with the pre-existing StrengthsElegant design. No server-side changes needed. The change is entirely additive and backward-compatible — existing Good documentation of exceptions. The inline comments on Consistent application. All Test coverage. Issues and Suggestions1. Implicit contract for
Suggestion: Promote the constraint to the # In ScopedForm or its __init__
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if settings.DEBUG:
has_scope_fieldset = any(
getattr(fs, 'fieldset_id', None) == 'scope'
for fs in getattr(self, 'fieldsets', [])
)
if not has_scope_fieldset:
import warnings
warnings.warn(
f"{self.__class__.__name__} uses ScopedForm but declares no "
"FieldSet with fieldset_id='scope'; HTMX partial swap will fail silently.",
stacklevel=2,
)2. No validation of The docstring for import re
def __init__(self, *items, name=None, fieldset_id=None):
if fieldset_id is not None and settings.DEBUG:
if not re.match(r'^[a-zA-Z][a-zA-Z0-9_-]*$', fieldset_id):
raise ValueError(f"fieldset_id {fieldset_id!r} is not a valid CSS identifier")
self.items = items
self.name = name
self.fieldset_id = fieldset_id3. In 4.
Test CoverageThe unit tests at
The tests are purely attribute-level (not end-to-end render tests), which is appropriate given that no template logic changed. SummaryThe implementation is clean, minimal, and correct. The two items worth acting on before merge are:
Neither is a blocker, but both would improve maintainability. All existing consumers are correctly wired. The code is ready to merge pending those considerations. |
|
Addressed the two actionable items. |
Adds an optional `id` parameter to `FieldSet` and a matching `hx_fieldset_id` parameter to `HTMXSelect`. When both are set, the widget emits `hx-select` and retargets `hx-target` to the named fieldset div rather than `#form_fields`, eliminating the full-form flash on large forms. The server always returns the complete form HTML; `hx-select` extracts only the target element on the client side, so no view or template changes are required. `hx-include` remains `#form_fields` so all field values are submitted and dependent-field resolution works correctly. Wires up `InterfaceForm` as the initial case: the 802.1Q Mode field now refreshes only the 802.1Q Switching fieldset. Closes #15165 partially — CableForm and ModuleTypeForm are candidates for a follow-up. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ypeForm Extends the HTMXSelect partial-swap pattern from InterfaceForm to two more forms: - CableForm: A Side and B Side termination selects now each target their own fieldset div (cable-side-a / cable-side-b) in cable_edit.html, so changing one side type no longer re-renders the other side. - ModuleTypeForm: the Profile selector targets the Profile and Attributes fieldset, which holds the dynamically generated attribute fields. Partial swap works correctly here because hx-include still sends the full form, so the server builds the complete attr_fields list before the client extracts just the target fieldset from the response. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ect forms Applies the hx_fieldset_id / FieldSet id pattern to all remaining model forms where partial swap is safe (i.e. the trigger field and all fields that change in response live within a single fieldset): - ScopedForm mixin: scope_type targets fieldset id=scope; all three consumers (ClusterForm, WirelessLANForm, PrefixForm) annotated - VLANGroupForm: scope_type / Scope fieldset - ServiceForm: parent_object_type / Application Service fieldset - VMInterfaceForm: mode / 802.1Q Switching fieldset (mirrors InterfaceForm) - CircuitTerminationForm: termination_type / Circuit Termination fieldset - CircuitGroupAssignmentForm: member_type / Group Assignment fieldset - VirtualCircuitTerminationForm: role / single fieldset - TunnelCreateForm: termination1_type and termination2_type each target their own First/Second Termination fieldsets independently - TunnelTerminationForm: type / single fieldset - EventRuleForm: action_type / Action fieldset Three forms intentionally left on full-form swap: - CustomFieldForm: type change adds/removes Validation/Related Object/ Choices fieldsets dynamically; partial swap would miss those additions - DataSourceForm: type change adds/removes Backend Parameters fieldset - VirtualMachineForm: type change populates defaults across both the Virtual Machine and Resources fieldsets Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CustomFieldForm, DataSourceForm, and VirtualMachineForm each have a type/kind selector that either adds/removes whole fieldsets dynamically or populates fields across multiple fieldsets. Partial fieldset swap would silently miss those changes, so they intentionally stay on the full hx-target='#form_fields' path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- HTMXSelect: add hx-swap=outerHTML when hx_fieldset_id is set, preventing progressive div nesting on repeated selections - FieldSet: rename id parameter to fieldset_id to avoid shadowing builtins; update all call sites across circuits, dcim, extras, ipam, virtualization, vpn, wireless - templatetags: use fieldset.fieldset_id directly instead of getattr fallback - ScopedForm: add comment documenting the fieldset_id contract for consumers - Add FieldSetTestCase and HTMXSelectTestCase unit tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- FieldSet.__init__: raise ValueError in DEBUG mode if fieldset_id is not a valid CSS identifier (must start with a letter) - ScopedForm.__init__: emit a warnings.warn in DEBUG mode when a subclass does not declare a FieldSet with fieldset_id='scope', preventing silent HTMX swap failures for future consumers - Add override_settings tests covering both the valid/invalid CSS identifier paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
503eaab to
a9dcec1
Compare
Conflicts resolved:
- circuits/forms/model_forms.py: keep hx_fieldset_id='circuit-termination'
on CircuitTerminationForm (feature branch had plain HTMXSelect())
- render_fieldset.html: combine both changes on the fieldset div —
id={{ fieldset_id }} from this branch and
role/aria-label from feature branch are complementary
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FieldSet: rename fieldset_id parameter to html_id (less redundant, more generic — it is an HTML id attribute, not fieldset-specific). HTMXSelect: separate the two concerns that were conflated in hx_target_id: - hx_include_id (new name for old hx_target_id): the form container whose data is submitted with the HTMX request — always #form_fields so that the server receives the full form state for dependent-field resolution. - hx_target_id (new name for old hx_fieldset_id): the element to swap and hx-select from the response — defaults to hx_include_id (full swap) or can be set to a specific fieldset for partial swaps. This makes Jeremy's intuition correct: you can now pass just hx_target_id='dot1q-switching' to target a specific fieldset without accidentally scoping hx-include to that fieldset too. Update all call sites, templates, templatetags, and tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2e1f599 to
680555d
Compare
The file was unintentionally modified by a local pre-commit hook that regenerated graphiql assets. Our changes are Python/template only and do not affect static bundles.
…add missing html_ids - Remove HTMXSelect from VirtualCircuitTerminationForm.role: the form renders identically regardless of role selection, so the widget was an errant copy-paste - Fix three comments that still read hx_fieldset_id (stale name) → hx_target_id in core, extras, and virtualization forms - Add html_id='scope' to PrefixBulkAddForm's Scope fieldset (was missing, unlike PrefixForm and PrefixBulkEditForm which already had it) - Add html_id='service' to ServiceCreateForm's Application Service fieldset (was missing, unlike the parent ServiceForm which already had it) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bctiemann
left a comment
There was a problem hiding this comment.
Thanks for the thorough review — addressed in the latest push:
- Dropped the errant
HTMXSelectfromVirtualCircuitTerminationForm.role - Fixed the three stale
hx_fieldset_idcomments →hx_target_id - Added
html_id='scope'toPrefixBulkAddForm's Scope fieldset andhtml_id='service'toServiceCreateForm's Application Service fieldset
Two open points worth a word:
Auto-generating slugified IDs for FieldSets: I lean toward keeping them explicit for this PR. Auto-generation is appealing, but it would require updating every hx_target_id value in every form and every HTMX hx-target attribute in templates to use the new fieldset-* prefix — a wider blast radius than seems right for a follow-up. Happy to do it as a separate cleanup if you'd prefer consistency over explicitness.
termination_type required=False: The underlying model field is a nullable FK (null=True, blank=True on CircuitTermination.termination_type), so a circuit termination can legitimately be saved with no termination target at all. The required=False corrects what was a latent mismatch with the model — the form was previously required=True by default even though the DB allows null. The form's clean() already handles the "type selected but no termination" case; the only new behaviour is allowing both to be blank.
Closes: #15165
Summary
Replaces the full
#form_fieldsswap with targeted per-fieldset swaps for allHTMXSelectfields where it is safe to do so, eliminating the visible flash on large forms when a select value changes.How it works
FieldSetgains an optionalfieldset_id=parameter that renders as the HTMLidon the fieldset<div>.HTMXSelectgains an optionalhx_fieldset_id=parameter. When set, the widget emitshx-selectand retargetshx-targetto the named fieldset instead of#form_fields.hx-includeremains#form_fieldsso all field values are submitted and server-side dependent-field resolution continues to work correctly.hx-selectextracts only the matching element on the client side.Forms updated
InterfaceFormmodedot1q-switchingVMInterfaceFormmodedot1q-switchingCableForma_terminations_type/b_terminations_typecable-side-a/cable-side-b(hand-rolled template)ModuleTypeFormprofileprofile-attributes(dynamic fieldset)ScopedFormmixinscope_typescope(applied toClusterForm,WirelessLANForm,PrefixForm)VLANGroupFormscope_typescopeServiceFormparent_object_typeserviceCircuitTerminationFormtermination_typecircuit-terminationCircuitGroupAssignmentFormmember_typecircuit-group-assignmentVirtualCircuitTerminationFormrolevirtual-circuit-terminationTunnelCreateFormtermination1_type/termination2_typetunnel-termination1/tunnel-termination2TunnelTerminationFormtypetunnel-terminationEventRuleFormaction_typeevent-rule-actionForms intentionally left on full-form swap
Three forms keep
HTMXSelect()withouthx_fieldset_idbecause changing their trigger field adds or removes entire fieldsets dynamically — a partial swap would silently miss those additions. Each is annotated with an inline comment explaining the constraint:CustomFieldForm—typechange dynamically adds/removes Validation, Related Object, and Choices fieldsets viaself.fieldsets = ...in__init__DataSourceForm—typechange conditionally appends a Backend Parameters fieldsetVirtualMachineForm—virtual_machine_typepopulates defaults across both the Virtual Machine and Resources fieldsetsNotes
FieldSet.idmust be a valid CSS identifier (start with a letter). The inline docstring notes this constraint.CableFormchange targets<div>elements indcim/htmx/cable_edit.htmldirectly (that form uses a hand-rolled HTMX template rather than the fieldset rendering system).ScopedBulkEditForm,CircuitTerminationBulkEditForm,VLANGroupBulkEditForm) already use an explicithx-selecttargeting#form_fieldsfor a different purpose and are unchanged.Test plan
InterfaceForm: change 802.1Q Mode — only the 802.1Q Switching fieldset updates; no full-form flashCableForm: change A Side type — only A Side updates; B Side is unaffectedCableForm: change B Side type independentlyModuleTypeForm: switch profile — only Profile & Attributes fieldset re-renders; dynamically added attribute fields appear correctlyutilitiestest suite (314 tests): all passdcimform tests (30 tests): all pass🤖 Generated with Claude Code
Addendum — Review feedback (Jun 9)
Two naming improvements made in response to Jeremy's review:
FieldSet:fieldset_id→html_idThe parameter name was redundant given it lives on a class already called
FieldSet. Renamed tohtml_idthroughout — rendering.py, form_helpers templatetag, render_fieldset.html, all call sites in forms, and tests.HTMXSelect: splithx_target_id/hx_fieldset_idintohx_include_id/hx_target_idJeremy asked why
hx_target_idcouldn't simply be overridden instead of needing a separatehx_fieldset_id. The reason: the originalhx_target_idcontrolled bothhx-include(the form data source) and the defaulthx-target. Overriding it would also scopehx-includeto just the fieldset, dropping the rest of the form data and breaking server-side dependent-field resolution.Fixed by separating the two concerns explicitly:
hx_include_idhx-include— which container's fields are submitted'form_fields'hx_target_idhx-target+hx-select— which element to swapNone(falls back tohx_include_id)Jeremy's instinct is now correct:
HTMXSelect(hx_target_id='dot1q-switching')does exactly what was expected —hx-includestays#form_fieldswhilehx-targetandhx-selectnarrow to#dot1q-switching.All call sites, tests, and the
ScopedFormruntime guard updated accordingly. The inline comments referencing the oldhx_fieldset_idname updated tohx_target_id.