Create contact form block

Created on 17 May 2013, about 12 years ago
Updated 30 January 2023, over 2 years ago

Problem/Motivation

The ability to select a contact category form is missing from the site wide contact form.

To reproduce:

  1. Install standard profile
  2. Add an additional contact category
  3. View /contact

What the will look like

What's missing

Proposed resolution

Introduce a block with a configurable list of contact forms

Remaining tasks

Update help text according UX review
Discuss enabling the Contact forms block automatically
Add tests

User interface changes

New block with a configurable set of links to other contact forms

API changes

no

Depends on the issue

#1477802: Shorter description for contact category weight
#599770: Clean up the contact forms listing UI: Allow to set the default category and weights on the listing page

Originally was:
Suggests to use tabs #1849158: Expose contact category-specific forms and/or their URLs somewhere

🐛 Bug report
Status

Needs work

Version

10.1

Component
Contact 

Last updated 4 days ago

Created by

🇬🇧United Kingdom alexpott 🇪🇺🌍

Live updates comments and jobs are added and updated live.
  • Usability

    Makes Drupal easier to use. Preferred over UX, D7UX, etc.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

  • Status changed to Needs review over 2 years ago
  • Status changed to RTBC over 2 years ago
  • 🇺🇸United States smustgrave

    Think this is ready to go. I worked on the tests #127 but not the solution.

    Question for the committer is if this will need a change record.

  • Status changed to Needs work over 2 years ago
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    At this point, I think this is a feature request, and the presence of 'Create' in the issue title reinforces that.

      I think its a good addition, but there's a bit to go still before I think its ready

    1. +++ b/core/modules/contact/config/schema/contact.schema.yml
      @@ -49,3 +49,11 @@ contact.settings:
      +      type: mapping
      +      label: 'Form'
      

      for a mapping the keys must be known, so should this instead be a sequence where the keys are the contact forms and the values are the weights?

    2. +++ b/core/modules/contact/contact.module
      @@ -37,6 +37,16 @@ function contact_help($route_name, RouteMatchInterface $route_match) {
      +      $output = '<p>' . t('Create separate contact forms for different purposes.') . '</p>';
      +      $output .= '<p>' . t('If you would like additional text to appear on the site-wide contact page, use a block. You can create and edit blocks on the <a href="@blocks">Blocks administration page</a>.', ['@blocks' => Url::fromRoute('block.admin_display')->toString()]) . '</p>';
      

      The remaining tasks says 'UX review' has that occurred for these new strings?

    3. +++ b/core/modules/contact/contact.module
      @@ -37,6 +37,16 @@ function contact_help($route_name, RouteMatchInterface $route_match) {
      +      if (\Drupal::moduleHandler()->moduleExists('block')) {
      +        $output .= '<p>' . t('Each form gets a link in the Contact forms block. You can enable this block on the <a href="@block">Block layout</a> page.', [
      

      i'm not sure why we check for the block module here when outside the if we already link to the block display page? Shouldn't both links be inside the if?

    4. +++ b/core/modules/contact/src/ContactFormListBuilder.php
      @@ -18,7 +18,7 @@ class ContactFormListBuilder extends ConfigEntityListBuilder {
      -    $header['selected'] = t('Selected');
      +    $header['default'] = t('Default');
           return $header + parent::buildHeader();
         }
       
      @@ -30,7 +30,7 @@ public function buildRow(EntityInterface $entity) {
      
      @@ -30,7 +30,7 @@ public function buildRow(EntityInterface $entity) {
           if ($entity->id() == 'personal') {
             $row['form'] = $entity->label();
             $row['recipients'] = t('Selected user');
      -      $row['selected'] = t('No');
      +      $row['default'] = t('No');
           }
           else {
             $row['form'] = $entity->toLink(NULL, 'canonical')->toString();
      @@ -40,7 +40,7 @@ public function buildRow(EntityInterface $entity) {
      
      @@ -40,7 +40,7 @@ public function buildRow(EntityInterface $entity) {
               '#context' => ['list_style' => 'comma-list'],
             ];
             $default_form = \Drupal::config('contact.settings')->get('default_form');
      -      $row['selected'] = ($default_form == $entity->id() ? t('Yes') : t('No'));
      +      $row['default'] = ($default_form == $entity->id() ? t('Yes') : t('No'));
      

      these changes feel out of scope.

    5. +++ b/core/modules/contact/src/Plugin/Block/ContactNavigationBlock.php
      @@ -0,0 +1,235 @@
      +    return ['cache_context.user.roles'];
      

      shouldn't this just be user.permissions?
      also I don't think the cache context is cache_context.user.roles, it should be just user.roles, so that indicates missing test coverage I think

    6. +++ b/core/modules/contact/src/Plugin/Block/ContactNavigationBlock.php
      @@ -0,0 +1,235 @@
      +      '#empty' => $this->t('There is no contact forms yet. Create one.'),
      

      Should this be 'there are no contact forms yet'?

      Should the 'create one' text link somewhere for users with appropriate permissions?

    7. +++ b/core/modules/contact/src/Plugin/Block/ContactNavigationBlock.php
      @@ -0,0 +1,235 @@
      +      if ($id == 'personal') {
      

      nit ===

    8. +++ b/core/modules/contact/src/Plugin/Block/ContactNavigationBlock.php
      @@ -0,0 +1,235 @@
      +      $form_state->setErrorByName('forms', $this->t('At least one category should be selected.'));
      

      We don't seem to have tests for this

    9. +++ b/core/modules/contact/src/Plugin/Block/ContactNavigationBlock.php
      @@ -0,0 +1,235 @@
      +      $links[$id] = $contact_form->toLink($contact_form->label(), 'canonical', [
      +        'set_active_class' => TRUE,
      +      ]);
      

      We can make use of named arguments here and avoid the first two arguments which are just the default

    10. +++ b/core/modules/contact/tests/src/Functional/ContactSitewideTest.php
      @@ -53,6 +53,8 @@ protected function setUp(): void {
      +    // cspell:ignore contactforms
      +    $this->drupalPlaceBlock('contact_navigation', ['id' => 'contactforms']);
      

      why not use something that doesn't trigger cspell instead of adding the ignore? `contact_forms` would probably do?

    11. +++ b/core/modules/contact/tests/src/Functional/ContactSitewideTest.php
      @@ -220,6 +223,21 @@ public function testSiteWideContact() {
      +    $this->assertSession()->statusCodeEquals(200);
      +    $edit = [
      +      'settings[forms][' . $name . '][display]' => TRUE,
      +    ];
      +    $this->submitForm($edit, 'Save block');
      +
      +    // Test that the contact form block appears.
      +    $this->drupalGet('contact');
      +    // cspell:ignore contactforms
      

      there's nothing here that is testing the quite involved logic around ordering of disabled items at the bottom etc that's present in the block form, so I think we should add tests, or simplify that logic

    Thanks for working on this again, let's keep up the momentum!

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Also we're missing calculateDependencies and onDependencyRemoval here

    i.e. the block should depend on the contact form, and should remove it from the selected options when the contact form is deleted

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    And yes to a change record to trumpet the new addition 📣

  • Status changed to Postponed about 2 months ago
  • 🇳🇿New Zealand quietone

    The Contact Module was approved for removal in 🌱 [Policy] Move Contact module to contrib Active .

    This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

    The deprecation work is in 📌 [meta] Tasks to deprecate the Contact module Active and the removal work in 📌 [12.x] [meta] Tasks to remove Contant module Active .

    Contact will be moved to a contributed project after the Drupal 12.x branch is open.

Production build 0.71.5 2024