Add fieldset formwidget

[mjauvin]

This formwidget wraps form elements with a <fieldset> HTML tag.

The fields are visually grouped but saved as if they were regular fields at the parent level.

The field's label config is used for the <fieldset> legend.

The Form widget's getSaveData() method now checks for a getFormFields() method on widgets in order to pull child fields from the widget.

The field config is akin to a nestedform:

myFieldSet:
  type: fieldset
  label: Grouped Fields
  form:
    fields:
      ...

Summary by CodeRabbit

• New Features

  • Adds a FieldSet form widget to embed a nested form section with an optional legend and an option to hide labels.

• Style

  • New themed styling for the FieldSet, including dark-mode support and improved tab visuals.

• Bug Fixes

  • Nested widget fields are now collected and saved correctly during form submission.

[damsfx]

@mjauvin How is the responsive design being managed?
Does the span left/right/auto or custom Css with span:storm still works ?

[mjauvin]

@mjauvin How is the responsive design being managed? Does the span left/right/auto or custom Css with span:storm still works ?

It's pretty much derived from a nestedform ... so it renders a form within the <fieldset> tag.

The main difference is that fields are exported to the parent when saving (does not use a JSON field).

[LukeTowers]

@mjauvin can you make a PR to the docs and include examples in the nestedform and fieldset sections as to when you should pick one vs the other?

[mjauvin]

@mjauvin can you make a PR to the docs and include examples in the nestedform and fieldset sections as to when you should pick one vs the other?

Done in wintercms/docs#258

[mjauvin]

This is now working great with forwidgets inside the fielset. It won't work at multiple levels, though (e.g. widget inside a nestedform inside the fieldset).

[LukeTowers]

@mjauvin if it won't work under certain conditions can we detect that and throw a systemexception when an unsupported configuration is detected?

[LukeTowers]

Given the changes to the Form widget getSaveData I'd really like to see some unit testing around that, do we have anything there right now or do we need to add coverage from scratch?

[mjauvin]

Given the changes to the Form widget getSaveData I'd really like to see some unit testing around that, do we have anything there right now or do we need to add coverage from scratch?

It doesn't change Form's getSaveData(), it adds a specific case for the FieldSet widget if present. Current behavior is not changed if no FieldSet is used.

[mjauvin]

@mjauvin if it won't work under certain conditions can we detect that and throw a systemexception when an unsupported configuration is detected?

Not sure it's worth processing all fields recursively to find potential issues down below...

[LukeTowers]

@mjauvin wouldn't all those fields have to be processed recursively anyways? What specific conditions cause the issue that you were talking about and what is the impact of that issue? Perhaps I didn't understand you fully the first time

[mjauvin]

There is no more issues, they were resolved by calling widget's getSaveValue() when going through the FieldSets' internal fields (see below):

            if ($widget instanceof FieldSet) {
                foreach ($widget->getFormFields() as $field) {
                    $parts = HtmlHelper::nameToArray($field->fieldName);
                    if (($value = $this->dataArrayGet($data, $parts)) !== null) {
                        switch ($field->type) {
                            case 'number':
                                $value = !strlen(trim($value)) ? null : (float) $value;
                                break;
                            case 'widget':
                                $value = $widget->getFormWidget($field->fieldName)->getSaveValue($value);
                                break;
                        }
                        $this->dataArraySet($result, $parts, $value);
                    }
                }
                continue;
            }

The only remaining "gap" is if og of those widgets itself has widgets, but the FieldSet shouldn't be used for deeply nested fields anyway.

[mjauvin]

@LukeTowers anything preventing this to get merged?

[LukeTowers]

@mjauvin if it only supports fields under form then we should just perform that merge ourselves so that the definition only provides fields.* instead of form.fields.*. If you can make that change to the code and docs then I should be able to review and merge.

[mjauvin]

@mjauvin if it only supports fields under form then we should just perform that merge ourselves so that the definition only provides fields.* instead of form.fields.*. If you can make that change to the code and docs then I should be able to review and merge.

Why would it be different than with the nestedform widget?