Work on #1491.

[mjordan]

Note: This PR replaces #777.

Github Issue: Islandora/documentation#1491

What does this Pull Request do?

Provides default values for the media name field, and for Image media, a default value for the Alt text field.

What's new?

Implementation of hook_form_alter() to populate the media name field, and an implementation of hook_field_widget_WIDGET_TYPE_form_alter() to populate the image Alt text field.

How should this be tested?

1. Check out this branch.
2. Clear your cache (e.g. drush cr)
3. Create a new Islandora object with the model "image" and add an image media. The image Name and Alt text fields should be automatically populated with the parent node's title.
4. Create a new Islandora object with a model different than "image" and add a media. The media's Name field should be automatically populated with the parent node's title.

Additional Notes:

• Does this change the interface, add a new feature, or otherwise change behaviours that would require updating documentation? Yes, improves the UX by providing sensible default values in form fields.
• Does this change add any new dependencies? No.
• Does this change require any other modifications to be made to the repository (ie. Regeneration activity, etc.)? No
• Could this change impact execution of existing code? Yes.

Interested parties

@Islandora/8-x-committers

[mjordan]

Bumping @Islandora/8-x-committers on this PR. @manez had approved the PR this one replaces (which I created against the wrong branch).

[jordandukart]

Hey @mjordan this seems like a sound thing to do but I think injecting JS might be a bit heavy handed. I think we can implement hook_field_widget_WIDGET_TYPE_form_alter here. If we add a new function to the #process array it will run after the widget's which appends on the additional elements. From there we could just change the #default_value of the alt element.

Example code:

function islandora_field_widget_image_image_form_alter(&$element, $form_state, $context) {
  $element['#process'][] = 'test';
}

function test($element, $form_state, $form) {
  if ($element['alt']['#access']) {
    $element['alt']['#default_value'] = 'testy';
  }
  return $element;
}

[mjordan]

@jordandukart thanks for the review and suggestion, I hadn't thought of that approach. I'll be away from my computer until next week but will try this then.

[jordandukart]

No worries, if you need to chat about anything ping me here or in Slack!

[mjordan]

@jordandukart I've incorporated your suggestion. Thanks!

[mjordan]

Travis failed on this:

FILE: /home/travis/build/Islandora/islandora/islandora.module

--------------------------------------------------------------------------------

FOUND 2 ERRORS AFFECTING 2 LINES

--------------------------------------------------------------------------------

 345 | ERROR | [x] Doc comment short description must start with a capital letter

 356 | ERROR | [x] Expected 1 blank line after function; 2 found

But I ran ./vendor/bin/phpcs --standard=./vendor/drupal/coder/coder_sniffer/Drupal web/modules/contrib/islandora/islandora.module prior to pushing and it didn't find these errors.

@Islandora/8-x-committers why's that?

[jordandukart]

Will try to get back to this today @mjordan!

[jordandukart]

Bit of a nit but you could just do an isset($params['edit']) or similarly this could be re-written as:

 if ($edit_param = \Drupal::request()->query->get('edit')) {
      $media_of_nid = $edit_param['field_media_of']['widget'][0]['target_id'];
      $node = \Drupal::entityTypeManager()->getStorage('node')->load($media_of_nid);
      if ($node) {
        $element['alt']['#default_value'] = $node->getTitle();
      }
    }

Not something that would stop me from approving this but just a code style thing.

[mjordan]

Cool, I've replaced if (count($params) > 0 && array_key_exists('edit', $params)) { with the much more concise if (isset($params['edit'])) {, but I'll keep the others.

Thanks again for the thorough review!

[jordandukart]

Looks good to me @mjordan!

[mjordan]

@Islandora/8-x-committers now that this has been reviewed, tested, and approved, can haz merge?