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
Comments
Comment #2
nakaza sora commentedComment #4
nakaza sora commentedI 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!
Comment #5
danrodHello @nakaza sora , could you fix the PHPCS issues, please?
https://git.drupalcode.org/issue/gifplayer-3558633/-/jobs/7428604
Comment #6
danrodComment #7
nakaza sora commentedHi @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.
Comment #8
danrodThanks @nakaza sora , I tested this and works as expected, I'll merge this to the 2.0.x branch.
Thanks again.
Comment #9
danrodComment #11
danrod