This module creates a bridge between other modules or themes in integrating external-referencing SVG icons defined in sprite sheets into Drupal.

This module provides a visual UI for end users to be able to select icons provided by extensions. This module provides no icons by itself.

The focus of this extension and how it differs from other icon modules available is that it provides improved developer experience when using external-referencing SVG icons technique specifically, while not compromising on the user interface as well. I have used a variation of this code on projects for custom themes I built, allowing selection of category icons and such for clients, while keeping an easy maintainable icon system in the theme via a build system.

Project link

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

Git instructions

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

PAReview checklist

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

Manual reviews of other projects

Comments

Wongjn created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: +PAreview: project created less than ten days ago
ankush_03’s picture

ankush_03
Location Gurgaon, India 🇮🇳
commented
Issue summary: View changes

Removing errors on page reported by pareview.

ankush_03’s picture

ankush_03
Location Gurgaon, India 🇮🇳
commented
Status: Needs review » Needs work

Thanks for your contribution !

Fix below issue reported by pareview.

Review of the 8.x-1.x branch (commit f54420f):
Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
DrupalPractice has found some issues with your code, but could be false positives.

FILE: .../pareview_temp/tests/modules/ex_icons_test/ex_icons_test.routing.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
7 | WARNING | Open page callback found, please add a comment before the
| | line why there is no access restriction
--------------------------------------------------------------------------

Time: 2.55 secs; Memory: 4Mb
This automated report was generated with PAReview.sh, your friendly project application review script.

FILE: ...000000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/README.md
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
142 | WARNING | Line exceeds 80 characters; contains 96 characters
--------------------------------------------------------------------------

FILE: .../site1101/web/vendor/drupal/pareviewsh/pareview_temp/ex_icons.module
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
70 | ERROR | [x] Equals sign not aligned with surrounding assignments;
| | expected 1 space but found 2 spaces
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...pareviewsh/pareview_temp/src/Plugin/Discovery/SvgSymbolDiscovery.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
89 | WARNING | Only string literals should be passed to t() where
| | possible
--------------------------------------------------------------------------

FILE: ...areview_temp/src/Plugin/Field/FieldFormatter/ExIconLinkFormatter.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
187 | ERROR | [x] Equals sign not aligned with surrounding assignments;
| | expected 1 space but found 2 spaces
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...sh/pareview_temp/src/Plugin/Field/FieldWidget/ExIconSelectWidget.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
59 | ERROR | [x] Equals sign not aligned with surrounding assignments;
| | expected 1 space but found 2 spaces
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...0/site1101/web/vendor/drupal/pareviewsh/pareview_temp/src/ExIcon.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
19 | WARNING | Only string literals should be passed to t() where
| | possible
--------------------------------------------------------------------------

Time: 3.79 secs; Memory: 6Mb

wongjn’s picture

wongjn commented
Status: Needs work » Needs review 
FILE: .../pareview_temp/tests/modules/ex_icons_test/ex_icons_test.routing.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 7 | WARNING | Open page callback found, please add a comment before the
   |         | line why there is no access restriction
--------------------------------------------------------------------------

I believe this is a false positive due to it being found in a test module.

FILE: ...000000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/README.md
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 142 | WARNING | Line exceeds 80 characters; contains 96 characters
--------------------------------------------------------------------------

This is due to a referenced URL.

FILE: .../site1101/web/vendor/drupal/pareviewsh/pareview_temp/ex_icons.module
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 70 | ERROR | [x] Equals sign not aligned with surrounding assignments;
    |       |     expected 1 space but found 2 spaces
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...areview_temp/src/Plugin/Field/FieldFormatter/ExIconLinkFormatter.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 187 | ERROR | [x] Equals sign not aligned with surrounding assignments;
     |       |     expected 1 space but found 2 spaces
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...sh/pareview_temp/src/Plugin/Field/FieldWidget/ExIconSelectWidget.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 59 | ERROR | [x] Equals sign not aligned with surrounding assignments;
    |       |     expected 1 space but found 2 spaces
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

These errors are due to a bug in the version of drupal/coder that PAReview uses: #2872220: False positive for equal sign alignment if only two variables are with = sign aligned

FILE: ...pareviewsh/pareview_temp/src/Plugin/Discovery/SvgSymbolDiscovery.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 89 | WARNING | Only string literals should be passed to t() where
    |         | possible
--------------------------------------------------------------------------


FILE: ...0/site1101/web/vendor/drupal/pareviewsh/pareview_temp/src/ExIcon.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 19 | WARNING | Only string literals should be passed to t() where
    |         | possible
--------------------------------------------------------------------------

I believe these code segments are tolerable due to the fact the code sections follow Drupal core code:

SvgSymbolDiscovery.php → \Drupal\Core\Plugin\Discovery\YamlDiscovery
ExIcon.php → \Drupal\breakpoint\Breakpoint, \Drupal\config_translation\ConfigNamesMapper

ankush_03’s picture

ankush_03
Location Gurgaon, India 🇮🇳
commented
Status: Needs review » Needs work
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review

Wongjn already replied in a comment.

wongjn’s picture

wongjn commented
Issue summary: View changes
Issue tags: +PAreview: review bonus
wongjn’s picture

wongjn commented
Issue summary: View changes
shaktik’s picture

shaktik
he/him
Primary language Hindi
Location New Delhi
commented 
FILE: ...s/ex_icons/tests/modules/ex_icons_test/ex_icons_test.routing.yml
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 7 | WARNING | Open page callback found, please add a comment before
   |         | the line why there is no access restriction
----------------------------------------------------------------------

Time: 284ms; Memory: 6Mb
wongjn’s picture

wongjn commented

Please see my previous comment where I address the PHP_CodeSniffer issues:

I believe this is a false positive due to it being found in a test module.

avpaderno’s picture

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

I reviewed the code and I didn't find any security issues. I am going to approve this application.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
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.

avpaderno’s picture

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

As for opting into security coverage for the project you used for this application, keep in mind you need to wait 10 days after the project has been created.
When the field description will not show New projects must wait 10 days before opting into security advisory coverage. Please take this time to review writing secure code and best practices. the field can be changed.

Status: Fixed » Closed (fixed)

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