Problem/Motivation

If we look at the contents of gifplayer.module file:

/**
 * Implements hook_help().
 */
// @phpstan-ignore-next-line
#[LegacyHook]
// phpcs:ignore
function gifplayer_help($route_name, RouteMatchInterface $route_match) {
  switch ($route_name) {
    case 'help.page.gifplayer':

      return '<p> ' . t(
        '@gifplayer is a module that defines a gifplayer configurable field
        types using the @gpl. Add as normal field to the fieldable entity and
        configure it.',
        [
          '@gifplayer' => Link::fromTextAndUrl(
            t('Gif Player'),
            Url::fromUri('https://www.drupal.org/project/gifplayer')
          )->toString(),
          '@gpl' => Link::fromTextAndUrl(
            t('Gif Player Library'),
            Url::fromUri('https://github.com/rubentd/gifplayer')
          )->toString(),
        ]
      ) . '</p>';
  }
}

/**
 * Implements hook_theme().
 */
// @phpstan-ignore-next-line
#[LegacyHook]
// phpcs:ignore
function gifplayer_theme($existing, $type, $theme, $path) {
  return [
    'gif_player' => [
      'variables' => [
        'gif_label' => NULL,
        'playon' => NULL,
        'wait' => NULL,
        'mode' => NULL,
        'width' => NULL,
        'height' => NULL,
        'gif' => NULL,
        'img_cover' => NULL,
      ],
    ],
  ];
}

/**
 * Implements hook_page_attachments().
 */
// @phpstan-ignore-next-line
#[LegacyHook]
// phpcs:ignore
function gifplayer_page_attachments(array &$attachments) {
  // Attach module library globally. Check it from module settings.
  $global_attach = \Drupal::config('gifplayer.settings')->get('library_attach');

  if ($global_attach) {
    $attachments['#attached']['library'][] = 'gifplayer/gifplayer-library';
    $attachments['#attached']['library'][] = 'gifplayer/main';
  }
}

This is duplicated code that is already implemented as service methods at /src/Hook/GifPlayerHooks.php so these need to converted to calls to these service methods.

Steps to reproduce

N/A

Proposed resolution

Look for the info above

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Issue fork gifplayer-3558633

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

danrod created an issue. See original summary.

nakaza sora’s picture

Assigned: Unassigned » nakaza sora

nakaza sora’s picture

Assigned: nakaza sora » Unassigned
Status: Active » Needs review

I have created a merge request for the following fixes related to OOP hook implementation:

Changes:
- Fixed the hook_help() implementation to delegate to the GifPlayerHooks service class
- Changed the method name gifPlayerFunctionsPageAttachments() to pageAttachments() for consistency
- Added proper dependency injection for ConfigFactoryInterface in the GifPlayerHooks constructor

Could you please review the merge request when you have time?

Thank you!

danrod’s picture

Hello @nakaza sora , could you fix the PHPCS issues, please?

https://git.drupalcode.org/issue/gifplayer-3558633/-/jobs/7428604

danrod’s picture

Status: Needs review » Needs work
nakaza sora’s picture

Status: Needs work » Needs review

Hi @danrod,
Thank you for pointing out the PHPCS issues and for the job link.
I’ve fixed the issues in the latest commit, so the job should be green now.
Please have a look when you have a moment.

danrod’s picture

Thanks @nakaza sora , I tested this and works as expected, I'll merge this to the 2.0.x branch.

Thanks again.

danrod’s picture

Status: Needs review » Reviewed & tested by the community

  • nakaza sora committed 6709362f on 2.0.x
    Issue #3558633: Fixes in the OOP Hooks Implementation
    
danrod’s picture

Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.