Problem/Motivation

In order to add certain colorbox gallery options available for other modules such as Video Embed Field module, we had to make gallery id generator code snippet a service so that it can be reusable.

Proposed resolution

Create new class to build the gallery id.
Add that class as a service.

Remaining tasks

Review

User interface changes

No changes

API changes

No changes

Data model changes

No changes

CommentFileSizeAuthor
#8 Captura de tela de 2020-04-23 12-59-25.png168.12 KBilgnerfagundes
#8 Captura de tela de 2020-04-23 13-01-56.png133.43 KBilgnerfagundes
#7 2935266-7.patch5.48 KBnitesh624
#3 build-gallery-id-review-1.patch5.69 KBpasan.gamage
build-gallery-id.patch5.55 KBpasan.gamage
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pasan.gamage created an issue. See original summary.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Needs review » Needs work
Issue tags: -code cleanup 
  1. +++ b/colorbox.theme.inc
@@ -7,6 +7,8 @@
+use Drupal\Core\Entity\ContentEntityInterface;
+use Drupal\Core\Field\FieldItemInterface;

    These aren't used anymore, so can be removed 

  2. +++ b/src/GalleryIdHelper.php
@@ -0,0 +1,95 @@
+class GalleryIdHelper {

    Needs a docblock 

  3. +++ b/src/GalleryIdHelper.php
@@ -0,0 +1,95 @@
+  /**
...
+  /**
...
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
+   * @param \Drupal\Core\Utility\Token $token

    Need comments 

  4. +++ b/src/GalleryIdHelper.php
@@ -0,0 +1,95 @@
+   * @param $entity
+   * @param $item
+   * @param $settings
+   *
+   * @return string

    needs to include type hints, short comment and comments 

  5. +++ b/src/GalleryIdHelper.php
@@ -0,0 +1,95 @@
+  function generateId(ContentEntityInterface $entity, FieldItemInterface $item, array $settings) {

    missing public visibility keyword

pasan.gamage’s picture

pasan.gamage CreditAttribution: pasan.gamage at PreviousNext commented
FileSize
build-gallery-id-review-1.patch5.69 KB

code review

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Needs work » Needs review
Neslee Canil Pinto’s picture

Neslee Canil Pinto
he/him
Primary language English
CreditAttribution: Neslee Canil Pinto at Specbee commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Patch Failed to apply.

nitesh624’s picture

nitesh624
he/him
Primary language English
Location Ranchi, India
CreditAttribution: nitesh624 as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Assigned: Unassigned » nitesh624
nitesh624’s picture

nitesh624
he/him
Primary language English
Location Ranchi, India
CreditAttribution: nitesh624 as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review
FileSize
2935266-7.patch5.48 KB

Reroll the patch

ilgnerfagundes’s picture

ilgnerfagundes CreditAttribution: ilgnerfagundes at CI&T commented
Status: Needs review » Reviewed & tested by the community
FileSize
Captura de tela de 2020-04-23 13-01-56.png133.43 KB
Captura de tela de 2020-04-23 12-59-25.png168.12 KB

Patch applies correctly

KapilV’s picture

KapilV CreditAttribution: KapilV as a volunteer and at OpenSense Labs commented

@nitesh624, thanks

This patch helps me.

nitesh624’s picture

nitesh624
he/him
Primary language English
Location Ranchi, India
CreditAttribution: nitesh624 as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Assigned: nitesh624 » Unassigned

Neslee Canil Pinto’s picture

Neslee Canil Pinto
he/him
Primary language English
CreditAttribution: Neslee Canil Pinto at Specbee commented
Status: Reviewed & tested by the community » Fixed

Committed. thanks👍🏻

Status: Fixed » Closed (fixed)

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