We need to port the D7 feeds_tamper Truncate Text plugin to D8 tamper.

CommentFileSizeAuthor
#5 interdiff-1-5.txt5.85 KBmitrpaka
#5 truncate_text-2976188-5.patch5.32 KBmitrpaka
#4 truncate_text-2976188-4.patch5.29 KBvijay.mayilsamy
#2 2976188_1.patch3.07 KBsivaramakrishnan
#2 plugin-page-3.png52.49 KBsivaramakrishnan
#2 plugin-page-2.png50.09 KBsivaramakrishnan
#2 plugin-page-1 .png54.97 KBsivaramakrishnan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jamesdixon created an issue. See original summary.

sivaramakrishnan’s picture

sivaramakrishnan CreditAttribution: sivaramakrishnan as a volunteer and commented
Status: Active » Needs review
FileSize
plugin-page-1 .png54.97 KB
plugin-page-2.png50.09 KB
plugin-page-3.png52.49 KB
2976188_1.patch3.07 KB

Please Apply the Here attached Patch and this patch for port the Truncate Text Plugin from D7 to D8.

ericgsmith’s picture

ericgsmith CreditAttribution: ericgsmith as a volunteer commented
Status: Needs review » Needs work

Hi @sivaramakrishnan - thanks for the patch. Looks like a good start.

This will need the settings definitions added to tamper.schema.yml and a test. A few minor comments below.

  1. +++ b/src/Plugin/Tamper/TruncateText.php
@@ -0,0 +1,86 @@
+      '#title' => t('Number of characters'),

    We should use $this->t for translated strings. 

  2. +++ b/src/Plugin/Tamper/TruncateText.php
@@ -0,0 +1,86 @@
+  public function tamper($data, TamperableItemInterface $item = NULL) {
...
+      $truncated_data = \Drupal\Component\Utility\Unicode::truncate($data, $this->getSetting(self::SETTING_NUM_CHAR), $this->getSetting(self::SETTING_WORDSAFE), $this->getSetting(self::SETTING_ELLIPSE));
...
+        $truncated_data = \Drupal\Component\Utility\Unicode::truncate($data, $this->getSetting(self::SETTING_NUM_CHAR), FALSE, $this->getSetting(self::SETTING_ELLIPSE));

    Can this method be simplified? If wordsafe is a boolean value, do we need the if statment or can we just return Unicode::truncate($data, $this->getSetting(self::SETTING_NUM_CHAR), $this->getSetting(self::SETTING_WORDSAFE), $this->getSetting(self::SETTING_ELLIPSE));

vijay.mayilsamy’s picture

vijay.mayilsamy CreditAttribution: vijay.mayilsamy as a volunteer and at Dialed In Design commented
FileSize
truncate_text-2976188-4.patch5.29 KB

Hi,

I have made all the above requested changes and Added the test cases.

Add Ellipses test case fails for some reason. Attached the patch here

@James, Could you please take a look and let me know what the problem is ?

Thanks
Vijay

mitrpaka’s picture

mitrpaka CreditAttribution: mitrpaka as a volunteer commented
Status: Needs work » Needs review
FileSize
truncate_text-2976188-5.patch5.32 KB
interdiff-1-5.txt5.85 KB

Re-rolled and updated latest patch.

jamesdixon’s picture

jamesdixon CreditAttribution: jamesdixon at Dialed In Design commented
Status: Needs review » Reviewed & tested by the community

Thanks for jumping in on this @mitrpaka.

Tests are still passing okay.

I have reviewed the code and @ericgsmith's suggestions have been covered from #3

Looks good to me!

  • ericgsmith committed 0f0fa3c on 8.x-1.x authored by mitrpaka 
    Issue #2976188 by sivaramakrishnan, mitrpaka, vijay.mayilsamy: Truncate...
ericgsmith’s picture

ericgsmith CreditAttribution: ericgsmith as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Thanks all! Apologies for the delay in committing this.

Status: Fixed » Closed (fixed)

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