I originally posted the module as a response to this forum post: https://www.drupal.org/forum/support/module-development-and-code-questio...

Then people started using it and I added many improvements.

This module provides a field formatter plugin for rendering File fields and Image fields as configurable links. A submodule provides a second field formatter for rendering Media reference fields as links to the media source.

In short, the formatters have these features:

  • Configurable link text with token integration
  • Configurable "target" attribute
  • Configurable "download" attribute
  • Automatic CSS class based on file type
  • Configurable additional CSS classes on link

How it's different

The file_download_link field formatter differ from core formatters for Image and File fields in that more attributes are configurable and in that the link text is much more flexible.

The file_download_link_media field formatter is clearly distinct from any core field formatter for Media reference fields. It allows a Media reference to be rendered directly as a link to the media source with several configuration options.

Project Link

https://www.drupal.org/project/file_download_link

Git instructions

git clone --branch 8.x-1.x https://git.drupalcode.org/project/file_download_link.git

PAReview checklist

https://pareview.sh/pareview/https-git.drupal.org-project-file_download_...

I consider the DrupalPractice issues to be false positives. The \Drupal calls are made to avoid a dependency on the token module. If the token module is enabled then one of its services is used. However, the module is still useful without token and I did not want to create a dependency.
I have changed how the token service gets called such that there are no \Drupal calls.

Comments

danflanagan8 created an issue. See original summary.

rksyravi’s picture

rksyravi
Primary language Hindi
Location New Delhi, 🇮🇳
commented

Hi @danflanagan8,

Thank you for contribution!!!
As you can see from PAReview, it says to use dependency injection instead of direct service call.

FILE: ...link_media/src/Plugin/Field/FieldFormatter/FileDownloadLinkMedia.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 100 | WARNING | \Drupal calls should be avoided in classes, use
     |         | dependency injection instead
--------------------------------------------------------------------------


FILE: ...h/pareview_temp/src/Plugin/Field/FieldFormatter/FileDownloadLink.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------
 125 | WARNING | \Drupal calls should be avoided in classes, use
     |         | dependency injection instead
 235 | WARNING | \Drupal calls should be avoided in classes, use
     |         | dependency injection instead
 279 | WARNING | \Drupal calls should be avoided in classes, use
     |         | dependency injection instead
--------------------------------------------------------------------------

Time: 1.4 secs; Memory: 4Mb
rksyravi’s picture

rksyravi
Primary language Hindi
Location New Delhi, 🇮🇳
commented
Status: Active » Needs work

I am changing the status to 'Needs work' after you resolve the errors, change the status to 'Need Review'.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes
danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Issue summary: View changes

Hi, rksyravi!
Thanks for starting the review process. As I wrote in the summary (I have now moved it to the end of the summary), I chose not to use dependency injection for the service provided by the token contrib module because I want to avoid having a dependency on that module. The field formatters still have several useful features even when tokens are not available.

Do you have a recommended approach to fix the errors without creating a dependency on token?

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Status: Needs work » Needs review
danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Issue summary: View changes
danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented

I have changed how the token service gets called such that there are no \Drupal calls. It still allows the formatters to be used if the token module is not installed. So the PAReview report is clean now.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Status: Needs review » Needs work
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

The previous status was correct, if you fixed what reported by reviewers. Needs work means you still need to fix the code.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented

Oops! Thanks, kiamlaluno. I got confused. I'll switch back to Needs Review.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Status: Needs work » Needs review
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Reviewed & tested by the community

Thanks for you contribution!

  1. '#title' => 'Example Token',: all user facing text must run through $this->t() for translation.
  2. $this->t('Classes:') . ' ' . $this->getSetting('custom_classes'): do not concatenate variables to translatable strings, use placeholder with $this->t() instead.
  3. getTokenWarningMarkup(): All user facing text must run through $thi->t() for translation. Please check all your UI strings.

Otherwise looks good to me!

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: Unassigned » avpaderno
Status: Reviewed & tested by the community » Fixed

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the dedicated reviewers as well.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented

Thank you, rksyravi!

Thank you, klausi!

Thank you, kiamlaluno!

Status: Fixed » Closed (fixed)

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