Add setting to move comment form after comments

Will need some form of upgrade path for the new settings.

Also tagging for sub-maintainer for their thoughts.

Will need some form of upgrade path for the new settings.

Upgrade path is not needed in this case. The new setting is disabled by default and does not affect existing sites.

The upgrade hook should set the new config to false.

If I go to the olivero settings, change nothing, click save, and do a config export I'll get a new change now because the schema is different.

Upgrade hook added

Looks good for me!

Thanks folks we still need a subsystem maintainer opinion here - would suggest pinging one of the olivero maintainers in #frontend in slack

Left some comments on the MR - thanks folks

Not olivero maintainer, but it's ok with me.

nod_ → closed merge request !7409

Committed 26e593a and pushed to 11.x. Thanks!

nod_ → reopened merge request !7409

feedback incomming

Added feedback to the MR.

The config changes look v nice... a couple of other thoughts...

We can add an update test to \Drupal\Tests\olivero\Functional\Update\OliveroPostUpdateTest
I think we can test the functionality in core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroCommentTest.js

I bet it needs change record, moreover it reminds me that we have comment type entity to store this kind of data

Personally I find comment field formatter or comment type is a better place to store position of the form.

Also it needs to revisit D6/7 migrations as there was related setting

@andypost pointed out that the comment field already includes a setting for 'form location' but that's only if its on the same page or a separate reply page. I think its ok for a theme to make a setting for how to position the form when its on the same page.

    $element['form_location'] = [
      '#type' => 'checkbox',
      '#title' => $this->t('Show reply form on the same page as comments'),
      '#default_value' => $settings['form_location'],
    ];

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.