LP#1424815: Cleanup/simplify perl and template

This commit attempts to cleanup and simplify the "Read more" catalog
feature by making the following changes:

  - Simplify perl util function
    - Remove extra blank lines
    - Remove first IF branch, since rindex already covers this case
    - Remove "rest" from return value, since Kathy's changed stopped
      using it
    - Add logic to address the FIXME comment
    - Add a few code comments

  - Simplify template
    - Only split a string if feature is turned on in config settings
    - Allow, but don't require, "trunc_length" on every marc note field
    - Add a few code comments

Signed-off-by: Remington Steed <rjs7@calvin.edu>
Signed-off-by: Kathy Lussier <klussier@masslnc.org>
Conflicts:
	Open-ILS/src/templates/opac/parts/record/contents.tt2

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm
index 3cbd754..ce324b1 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Util.pm
@@ -202,10 +202,6 @@ sub init_ro_object_cache {
         return (grep {$_->shortname eq $sn} @$list)[0];
     };
 
-
-
-
-
     # Turns one string into two for long text strings
     $locale_subs->{split_for_accordion} = sub {
         my $string = shift;
@@ -214,36 +210,18 @@ sub init_ro_object_cache {
         return unless defined $string && defined $trunc_length;
         return if $trunc_length > length $string;
 
-        my $short;
-        my $rest;
-        # find non-whitespace from end
-        # 0-BASED!
-
-        if ((substr $string, $trunc_length, 1) =~ /\s/) {
-            # Next character is whitespace - we're good; split there
-
-            $short = substr $string, 0, $trunc_length;
-            $rest  = substr $string, $trunc_length;
+        # split on the nearest space at or before trunc_length
+        my $loc = rindex $string, ' ', $trunc_length;
+        # if no spaces are found, split in the middle of the text
+        if ($loc == -1) {
+            $loc = $trunc_length;
         }
-        else {
-            # find the space nearest the end of our (not-yet) truncated string
-            my $loc = rindex $string, ' ', $trunc_length;
 
-            # FIXME: if $loc = -1 (can't truncate - no spaces found)
-            $short = substr $string, 0, $loc;
-            $rest = substr $string, $loc;
-        }
+        my $short = substr $string, 0, $loc;
 
-        return ($short, $rest, $string);
+        return ($short, $string);
     };
 
-
-
-
-
-
-
-
     $locale_subs->{aouct_tree} = sub {
 
         # fetch the org unit tree

diff --git a/Open-ILS/src/templates/opac/parts/record/contents.tt2 b/Open-ILS/src/templates/opac/parts/record/contents.tt2
index 5770ceb..712be1c 100644 (file)
--- a/Open-ILS/src/templates/opac/parts/record/contents.tt2
+++ b/Open-ILS/src/templates/opac/parts/record/contents.tt2
@@ -1,264 +1,218 @@
 [%-
 #provide a default value in case truncation length is unset in config.tt2
-
 default_contents_length = contents_truncate_length || 100;
 
+# A 'trunc_length' member can be added to any note type in the hash below to
+# individually control the length of the "Read more" cutoff point. An example
+# is commented out in the "General Note" type.
+
 contents =  [
     {
         display_field => 'general_note',
         label => l('General Note: '),
-        trunc_length => default_contents_length,
+        #trunc_length => 100,
         noteid => l('generalnote'),
         xpath => '//*[@tag="500"]'
     }, {
         label => l('With Note: '),
-        trunc_length => default_contents_length,
         noteid => l('withnote'),
         xpath => '//*[@tag="501"]'
     }, {
         display_field => 'thesis',
         label => l('Dissertation Note: '),
-        trunc_length => default_contents_length,
         noteid => l('dissertationnote'),
         xpath => '//*[@tag="502"]'
     }, {
         display_field => 'bibliography',
         label => l('Bibliography, etc. Note: '),
-        trunc_length => default_contents_length,
         noteid => l('bibnote'),
         xpath => '//*[@tag="504"]'
     }, {
         display_field => 'toc',
         label => l('Formatted Contents Note: '),
-        trunc_length => default_contents_length,
         noteid => l('contentsnote'),
         xpath => '//*[@tag="505"]'
     }, {
         label => l('Restrictions on Access Note: '),
-        trunc_length => default_contents_length,
         noteid => l('accessnote'),
         xpath => '//*[@tag="506"]'
     }, {
         label => l('Scale Note for Graphic Material: '),
-        trunc_length => default_contents_length,
         noteid => l('scalenote'),
         xpath => '//*[@tag="507"]'
     }, {
         display_field => 'production_credits',
         label => l('Creation/Production Credits Note: '),
-        trunc_length => default_contents_length,
         noteid => l('creationnote'),
         xpath => '//*[@tag="508"]'
     }, {
         label => l('Citation/References Note: '),
-        trunc_length => default_contents_length,
         noteid => l('citationnote'),
         xpath => '//*[@tag="510"]'
     }, {
         display_field => 'performers',
         label => l('Participant or Performer Note: '),
-        trunc_length => default_contents_length,
         noteid => l('performernote'),
         xpath => '//*[@tag="511"]'
     }, {
         label => l('Type of Report and Period Covered Note: '),
-        trunc_length => default_contents_length,
         noteid => l('reporttypenote'),
         xpath => '//*[@tag="513"]'
     }, {
         label => l('Data Quality Note: '),
-        trunc_length => default_contents_length,
         noteid => l('dataqualitynote'),
         xpath => '//*[@tag="514"]'
     }, {
         label => l('Numbering Peculiarities Note: '),
-        trunc_length => default_contents_length,
         noteid => l('numberingnote'),
         xpath => '//*[@tag="515"]'
     }, {
         label => l('Type of Computer File or Data Note: '),
-        trunc_length => default_contents_length,
         noteid => l('filetypenote'),
         xpath => '//*[@tag="516"]'
     }, {
         label => l('Date/Time and Place of an Event Note: '),
-        trunc_length => default_contents_length,
         noteid => l('eventnote'),
         xpath => '//*[@tag="518"]'
     }, {
         display_field => 'abstract',
         label => l('Summary, etc.: '),
-        trunc_length => default_contents_length,
         noteid => 'summarynote',
         xpath => '//*[@tag="520"]'
     }, {
         label => l('Target Audience Note: '),
-        trunc_length => default_contents_length,
         noteid => l('audiencenote'),
         xpath => '//*[@tag="521"]'
     }, {
         label => l('Geographic Coverage Note: '),
-        trunc_length => default_contents_length,
         noteid => l('geographicnote'),
         xpath => '//*[@tag="522"]'
     }, {
         label => l('Preferred Citation of Described Materials Note: '),
-        trunc_length => default_contents_length,
         noteid => l('prefcitationnote'),
         xpath => '//*[@tag="524"]'
     }, {
         label => l('Supplement Note: '),
-        trunc_length => default_contents_length,
         noteid => l('suppnote'),
         xpath => '//*[@tag="525"]'
     }, {
         label => l('Study Program Information Note: '),
-        trunc_length => default_contents_length,
         noteid => l('studynote'),
         xpath => '//*[@tag="526"]'
     }, {
         label => l('Additional Physical Form available Note: '),
-        trunc_length => default_contents_length,
         noteid => l('physformnote'),
         xpath => '//*[@tag="530"]'
     }, {
         label => l('Reproduction Note: '),
-        trunc_length => default_contents_length,
         noteid => l('repronote'),
         xpath => '//*[@tag="533"]'
     }, {
         label => l('Original Version Note: '),
-        trunc_length => default_contents_length,
         noteid => l('origvernote'),
         xpath => '//*[@tag="534"]'
     }, {
         label => l('Location of Originals/Duplicates Note: '),
-        trunc_length => default_contents_length,
         noteid => l('originalsnote'),
         xpath => '//*[@tag="535"]'
     }, {
         label => l('Funding Information Note: '),
-        trunc_length => default_contents_length,
         noteid => l('fundingnote'),
         xpath => '//*[@tag="536"]'
     }, {
         label => l('System Details Note: '),
-        trunc_length => default_contents_length,
         noteid => l('sysdetailsnote'),
         xpath => '//*[@tag="538"]'
     }, {
         label => l('Terms Governing Use and Reproduction Note: '),
-        trunc_length => default_contents_length,
         noteid => l('termsofusenote'),
         xpath => '//*[@tag="540"]'
     }, {
         label => l('Immediate Source of Acquisition Note: '),
-        trunc_length => default_contents_length,
         noteid => l('acqnote'),
         xpath => '//*[@tag="541"]'
     }, {
         label => l('Information Relating to Copyright Status: '),
-        trunc_length => default_contents_length,
         noteid => l('copyrightnote'),
         xpath => '//*[@tag="542"]'
     }, {
         label => l('Location of Other Archival Materials Note: '),
-        trunc_length => default_contents_length,
         noteid => l('archivalnote'),
         xpath => '//*[@tag="544"]'
     }, {
         label => l('Biographical or Historical Data: '),
-        trunc_length => default_contents_length,
         noteid => l('bionote'),
         xpath => '//*[@tag="545"]'
     }, {
         label => l('Language Note: '),
-        trunc_length => default_contents_length,
         noteid => l('langnote'),
         xpath => '//*[@tag="546"]'
     }, {
         label => l('Former Title Complexity Note: '),
-        trunc_length => default_contents_length,
         noteid => l('formertitlenote'),
         xpath => '//*[@tag="547"]'
     }, {
         label => l('Issuing Body Note: '),
-        trunc_length => default_contents_length,
         noteid => l('issuingbodynote'),
         xpath => '//*[@tag="550"]'
     }, {
         label => l('Entity and Attribute Information Note: '),
-        trunc_length => default_contents_length,
         noteid => l('entitynote'),
         xpath => '//*[@tag="552"]'
     }, {
         label => l('Cumulative Index/Finding Aids Note: '),
-        trunc_length => default_contents_length,
         noteid => l('indexnote'),
         xpath => '//*[@tag="555"]'
     }, {
         label => l('Information About Documentation Note: '),
-        trunc_length => default_contents_length,
         noteid => l('documentationnote'),
         xpath => '//*[@tag="556"]'
     }, {
         label => l('Ownership and Custodial History: '),
-        trunc_length => default_contents_length,
         noteid => l('ownershipnote'),
         xpath => '//*[@tag="561"]'
     }, {
         label => l('Copy and Version Identification Note: '),
-        trunc_length => default_contents_length,
         noteid => l('copyvernote'),
         xpath => '//*[@tag="562"]'
     }, {
         label => l('Binding Information: '),
-        trunc_length => default_contents_length,
         noteid => l('bindingnote'),
         xpath => '//*[@tag="563"]'
     }, {
         label => l('Case File Characteristics Note: '),
-        trunc_length => default_contents_length,
         noteid => l('casefilenote'),
         xpath => '//*[@tag="565"]'
     }, {
         label => l('Methodology Note: '),
-        trunc_length => default_contents_length,
         noteid => l('methodologynote'),
         xpath => '//*[@tag="567"]'
     }, {
         label => l('Linking Entry Complexity Note: '),
-        trunc_length => default_contents_length,
         noteid => l('linkingnote'),
         xpath => '//*[@tag="580"]'
     }, {
         label => l('Publications About Described Materials Note: '),
-        trunc_length => default_contents_length,
         noteid => l('publicationsnote'),
         xpath => '//*[@tag="581"]'
     }, {
         label => l('Action Note: '),
-        trunc_length => default_contents_length,
         noteid => l('actionnote'),
         xpath => '//*[@tag="583"]'
     }, {
         label => l('Accumulation and Frequency of Use Note: '),
-        trunc_length => default_contents_length,
         noteid => l('accumulationnote'),
         xpath => '//*[@tag="584"]'
     }, {
         label => l('Exhibitions Note: '),
-        trunc_length => default_contents_length,
         noteid => l('exhibitionsnote'),
         xpath => '//*[@tag="585"]'
     }, {
         label => l('Awards Note: '),
-        trunc_length => default_contents_length,
         noteid => l('awardsnote'),
         xpath => '//*[@tag="586"]'
     }, {
         label => l('Source of Description Note: '),
-        trunc_length => default_contents_length,
         noteid => l('descsourcenote'),
         xpath => '//*[@tag="588"]'
     } 
@@ -282,11 +236,12 @@ BLOCK render_contents;
         END;
         total_contents = all_content.join(" ").replace('\s+$', '');
         %] [% "<div class='content_field'>"; total_contents | html ; "</div>";
-            IF total_contents.length > cont.trunc_length; 
-              # need to chop!
-              blah = ctx.split_for_accordion(total_contents, cont.trunc_length);
-              cont.short = blah.0;
-              cont.long  = blah.2;
+            # if string is long enough, truncate for use with "Read more" link
+            read_more_length = cont.exists('trunc_length') ? cont.trunc_length : default_contents_length;
+            IF total_contents.length > read_more_length AND truncate_contents AND ctx.want_jquery;
+                split_pieces = ctx.split_for_accordion(total_contents, read_more_length);
+                cont.short = split_pieces.0;
+                cont.long  = split_pieces.1;
             END;
         FOREACH link880 IN graphics;
             '<div class="graphic880"' _ link880.dir _ '>';
@@ -310,7 +265,7 @@ BLOCK render_all_contents;
 <tr>
     <td class='rdetail_content_type'>[% cont.label %]</td>
     <td class='rdetail_content_value' property='keywords'>
-[% IF truncate_contents AND ctx.want_jquery AND cont.short.length AND cont.trunc_length %]
+[% IF truncate_contents AND ctx.want_jquery AND cont.short.length %]
 <noscript>
    <style type="text/css">
       .show_truncated {display:none;}