wintercms / winter

Free, open-source, self-hosted CMS platform based on the Laravel PHP Framework.
https://wintercms.com
MIT License
1.38k stars 194 forks source link

FormBuilder form_submit() not working as expected #864

Closed arvislacis closed 1 year ago

arvislacis commented 1 year ago

Winter CMS Build

dev-develop

PHP Version

8.1

Database engine

SQLite

Plugins installed

No response

Issue description

First of all, maybe this is not issue/bug but is related to https://github.com/wintercms/meta/blob/master/l9-upgrade-notes.md#html changes..., although clarification there suggests that it applies more for extended plugin classes than form_... calls in Twig templates.

Anyways, in WinterCMS 1.1.8 it was possible to use {{ form_submit('Apply', {'class': 'btn btn-primary'}) }} and render valid submit button with following output <input class="btn btn-primary" type="submit" value="Apply"> but after upgrading to WinterCMS 1.2.x-dev the output is only <input class="btn btn-primary">. In other words, submit type of input field and also value passed with the first parameter is missing in the rendered output.

Looks like issue was introduced with https://github.com/wintercms/storm/commit/2ef32346682c768a9d457c4dba6796f34b644eab#diff-ec32bc4f8918dddd6340e4e341849935c6765bdfba774c75b7553376a6109e2eR274 (@bennothommo)

Steps to replicate

In Twig file (page or component .html) create form and use {{ form_submit('Apply', {'class': 'btn btn-primary'}) }} or reduced version of that - {{ form_submit('Apply') }}. The rendered input field will not have submit type and also passed value "Apply" is missing (from https://wintercms.com/docs/services/html#buttons it's expected that the passed value is text which needs to be shown on the submit button).

Workaround

As an alternative solution it is possible to use:

arvislacis commented 1 year ago

@mjauvin I already mentioned that commit and even possibly problematic line number in my initial issue message, FYI.

ericp-mrel commented 1 year ago

The last working version was in v1.1, before the types were added in v1.2 (02c0c9b1e20c233619afcf126487bcf0e9a97ce3)

Extracting these lines so they're outside of the ! empty($value)) if statement fixed the problem (at least for the submit method, haven't tested for any other possible regressions). That way merging the value, type, and id attributes isn't dependent on the name being defined.

    public function input(string $type, ?string $name = null, ?string $value = null, array $options = []): string
    {
        if (!isset($options['name'])) {
            $options['name'] = $name;
        }

        $id = null;

        if (!empty($name)) {
            // We will get the appropriate value for the given field. We will look for the
            // value in the session for the value in the old input data then we'll look
            // in the model instance if one is set. Otherwise we will just use empty.
            $id = $this->getIdAttribute($name, $options);

            if (!in_array($type, $this->skipValueTypes)) {
                $value = $this->getValueAttribute($name, $value);
            }
        }

        // Once we have the type, value, and ID we can merge them into the rest of the
        // attributes array so we can convert them into their HTML attribute format
        // when creating the HTML element. Then, we will return the entire input.
        $merge = compact('type', 'value', 'id');

        $options = array_filter(array_merge($options, $merge), function ($item) {
            return !is_null($item);
        });

        return '<input' . $this->html->attributes($options) . '>';
    }
bennothommo commented 1 year ago

I'm working on a fix for this now. :)