refs #19281 [fid_bbi] improve get-it-box

* Fix most missing/duplicate get-it links.
* All fallback text if no info available.
* Use external link view helper for external links.
* Update "no resolver links" translation.
* Clean up template code.
* Replace preg_match_all with strpos and add TODO
* Fix HTML, use <div> and <p> appropriately
* Indent links
* Add margins between elements
* Remove margin below last element

themes/fid_bbi/languages/de.ini

@@ -62,6 +62,8 @@ home_tagline = Das Nachweisportal für die <br>Buch-, Biblio­theks- und Informa
 
 licenses_BookHistoryOnline = Bibliographie "Book History Online" bei brill.com
 
+no_resolver_links = Nicht online verfügbar.
+
 offcanvas-toggler-search-tips = Suchtipps anzeigen
 
 search_in_resource = In dieser Datenquelle suchen

themes/fid_bbi/languages/en.ini

@@ -33,5 +33,7 @@ home_tagline = The Main Research Catalogue for<br> Book Studies, Library and Inf
 
 licenses_BookHistoryOnline = "Book History Online" bibliography hosted at brill.com
 
+no_resolver_links = Not available online.
+
 search_in_resource = Search in this data source
 search_in_specific_resource = Search in "%%resource%%"

themes/fid_bbi/scss/base/common.scss

@@ -34,7 +34,7 @@ a {
     color: $link-color;
     cursor: pointer;
 
-    @include hover() {
+    @include hover {
       color: $link-hover-color;
     }
   }
@@ -117,10 +117,6 @@ dt {
   &:last-of-type {
     margin-bottom: 0;
   }
-
-  dl.-narrow & {
-    width: g(6);
-  }
 }
 
 figcaption {
@@ -304,5 +300,5 @@ video {
 }
 
 .hidden {
-  display: none !important;
+  display: none;
 }

themes/fid_bbi/scss/blocks/record.scss

@@ -57,17 +57,9 @@
     margin: 0 0 g(.5);
   }
 
-  .link-with-icon {
-    margin-left: 0; // unindent icons
-  }
-
   .unstyled-list {
     margin-bottom: g(.5);
 
-    &:last-child {
-      margin-bottom: 0;
-    }
-
     li + li {
       margin-top: 0;
     }
@@ -147,6 +139,17 @@
       margin-bottom: g(.5);
     }
   }
+
+  // Ensure there is no margin below the last element
+  // TODO: Check and adapt after holdingsils template has been refactored
+  > :last-child > :last-child {
+    margin-bottom: 0;
+
+    p:last-child,
+    ul:last-child {
+      margin-bottom: 0;
+    }
+  }
 }
 
 .record table,

themes/fid_bbi/templates/RecordTab/holdingsils.phtml

@@ -29,7 +29,7 @@ $isFree = in_array('Free', $this->driver->tryMethod('getFacetAvail'));
 
 <div class="record_getitbox">
   <?php if ($user && $getitConfig['bossData']): ?>
-    <p class="boss-data-true"></p>
+    <div class="boss-data-true"></div>
   <?php endif; ?>
 
   <?php if ($openUrlActive && $getitConfig['showArticleButton']): ?>
@@ -39,8 +39,9 @@ $isFree = in_array('Free', $this->driver->tryMethod('getFacetAvail'));
   <?php if ($getitConfig['showOrderButton'] && $user): ?>
     <p>
       <a
+        href="<?=$this->url('record-fidpda', array('id' => $id))?>"
+        rel="nofollow"
         data-lightbox
-        href="<?=$this->url('record-fidpda', array('id' => $id))?>" rel="nofollow"
       >
         <?=$this->transEsc('getit_order')?>
       </a>
@@ -50,8 +51,9 @@ $isFree = in_array('Free', $this->driver->tryMethod('getFacetAvail'));
   <?php if ($getitConfig['showPartCopyButton'] && $user): ?>
     <p>
       <a
+        href="<?=$this->url('record-fidsubitopartialcopy', array('id' => $id))?>"
+        rel="nofollow"
         data-lightbox
-        href="<?=$this->url('record-fidsubitopartialcopy', array('id' => $id))?>" rel="nofollow"
       >
         <?=$this->transEsc('getit_partcopy')?>
       </a>
@@ -86,7 +88,7 @@ $isFree = in_array('Free', $this->driver->tryMethod('getFacetAvail'));
     <?php foreach ($onlineUrls as $current): ?>
       <li>
         <a class="link-with-icon" href="<?=$this->escapeHtmlAttr($this->proxyUrl($current['link']))?>">
-          <?=$this->icon('arrow-right')?>
+          <?=$this->icon('small/external-link')?>
           <?=$this->escapeHtml($current['text'])?>
         </a>
       </li>
@@ -111,14 +113,14 @@ $(document).ready(function() {
     method: 'GET',
     data: {id: recordId, source: recordSource}
   }).done(function(response) {
-      if (response.data.isISIL) {
-        $('.boss-data-true').html(response.data.html);
-      } else {
-        $('.boss-data-false').html(response.data.html);
-        $('.boss-notice').addClass('hidden');
-      }
-      // Use to register new lightbox window networknotice
-      VuFind.lightbox.bind();
+    if (response.data.isISIL) {
+      $('.boss-data-true').html(response.data.html);
+    } else {
+      $('.boss-data-false').html(response.data.html);
+      $('.boss-notice').addClass('hidden');
+    }
+    // Use to register new lightbox window networknotice
+    VuFind.lightbox.bind();
   });
 });
 JS;
@@ -138,14 +140,14 @@ JS;
 
 <?php if ($openUrlActive): ?>
   <div class="getitbox-notices">
-    <p class="openUrlPrint"></p>
+    <div class="openUrlPrint"></div>
   </div>
 <?php endif; ?>
 
 <?php if (!empty($getitConfig['notice'])): ?>
   <div class="getitbox-notices">
     <?php if ($user && $getitConfig['bossData']): ?>
-      <p class="boss-data-false"></p>
+      <div class="boss-data-false"></div>
     <?php endif; ?>
 
     <?php
@@ -154,13 +156,16 @@ JS;
       . '" data-lightbox>' . $this->transEsc('getit_text_3_2') . '</a>';
     ?>
     <?php if ($getitConfig['hideNotice']): ?><div class="boss-notice"><?php endif; ?>
-      <?=sprintf($getitConfig['notice'], $linkHtml)?>
+      <p><?=sprintf($getitConfig['notice'], $linkHtml)?></p>
     <?php if ($getitConfig['hideNotice']): ?></div><?php endif; ?>
   </div>
 <?php elseif (!$getitConfig['isAiSidRecord']): ?>
   <div class="getitbox-notices resolver">
     <?php if ($user && $getitConfig['bossData']): ?>
-      <p class="boss-data-false"></p>
+      <div class="boss-data-false"></div>
+    <?php elseif (!$openUrlActive): ?>
+      <?php // TODO: Please check if we can replace this with a more helpful message. ?>
+      <p><?=$this->translate('getit_text_default'); ?></p>
     <?php endif; ?>
   </div>
 <?php endif; ?>

themes/fid_bbi/templates/RecordTab/worldcat.phtml

 <!-- fid_bbi: recordtab - worldcat -->
 <? $script = <<<JS
   $(document).ready(function() {
-      var recordId = $('.hiddenId').val();
-      var recordSource = $('.hiddenSource').val();
-      // console.log({id: recordId, source: recordSource});
-      $.ajax({
-        dataType: 'json',
-        url: VuFind.path + '/AJAX/JSON?method=getWorldCat',
-        method: 'GET',
-        data: {id: recordId, source: recordSource}
-      }).done(function(response) {
-          $('.worldcat-data').html(response.data.html);
-          $('.worldcat-headline').show();
-      });
+    var recordId = $('.hiddenId').val();
+    var recordSource = $('.hiddenSource').val();
+    // console.log({id: recordId, source: recordSource});
+    $.ajax({
+      dataType: 'json',
+      url: VuFind.path + '/AJAX/JSON?method=getWorldCat',
+      method: 'GET',
+      data: {id: recordId, source: recordSource}
+    }).done(function(response) {
+        $('.worldcat-data').html(response.data.html);
+        $('.worldcat-headline').show();
     });
+  });
 JS;
 ?>
-<!-- renders holdingstab contents here until we know exactly how it shall be designed -->
 <?=$this->inlineScript(\Zend\View\Helper\HeadScript::SCRIPT, $script, 'SET');?>
+
 <div class="worldcat-headline" style="display: none"><?=$this->translate('worldcat_availability')?></div>
 <div class="worldcat-data"></div>
+
 <?php
-/* #16470: build link with oclc number - GG */
+// #16470: build link with oclc number - GG
 $oclc = $this->driver->tryMethod('getOCLC');
 if (!empty($oclc)): ?>
-<div>
-  <?php
-  $url = sprintf('https://www.worldcat.org/oclc/%s', $oclc[0]);
-  ?>
-  <a href="<?=$url?>" target="_blank"><?=$this->transEsc('Link to WorldCat record')?></a>
-</div>
+  <p>
+    <?php $url = sprintf('https://www.worldcat.org/oclc/%s', $oclc[0]); ?>
+    <?=$this->externalLink($url, $this->translate('Link to WorldCat record'))?>
+  </p>
 <?php endif; ?>
 <!-- fid_bbi: recordtab - worldcat - END -->

themes/fid_bbi/templates/ajax/boss-false.phtml

@@ -9,7 +9,7 @@
  * @author Tobias Schäfer <ts@aspectis.net>
  */
 ?>
-<div class="boss-false">
+<p class="boss-false">
   <?php if ($this->homeLibrary): ?>
     <?=$this->translate('boss_default')?>
   <?php else: ?>
@@ -29,5 +29,5 @@
       <?=$this->translate('boss_noISXNZBD')?>
     <?php endif; ?>
   <?php endif; ?>
-</div>
+</p>
 <!-- fid_bbi: ajax - boss-false - END -->

themes/fid_bbi/templates/ajax/boss-true.phtml

@@ -9,7 +9,7 @@
  * @author Tobias Schäfer <ts@aspectis.net>
  */
 ?>
-<div class="boss-true">
+<p class="boss-true">
   <?php if ($this->homeLibrary): ?>
     <?=$this->translate('boss_default')?>
   <?php else: ?>
@@ -17,13 +17,14 @@
       <?php $linkText = $this->icon('external-link') . $this->transEsc('boss_true'); ?>
       <?=$this->externalLink($this->url, $linkText, ['class' => 'link-with-icon'])?>
       <?php if (!empty($this->callnumber)): ?>
-        <div class="pda-part-text">
+        <br>
+        <span class="pda-part-text">
           <?=$this->transEsc('boss_callnumber')?>: <?=implode(', ', $this->callnumber)?>
-        </div>
+        </span>
       <?php endif; ?>
     <?php else: ?>
       <?=$this->translate('boss_noISXNZBD')?>
     <?php endif; ?>
   <?php endif; ?>
-</div>
+</p>
 <!-- fid_bbi: ajax - boss-true - END -->

themes/fid_bbi/templates/ajax/resolverLinks-print.phtml

@@ -4,8 +4,7 @@
  * This is a modified copy of themes/bbi/templates/ajax/resolverLinks-print
  * Changes for FID BBI:
  * - change location HTML, remove breaks
- *
- * TODO: THIS IS A WORK IN PROGRESS
+ * - use if/else instead of variable
  *
  * @author Tobias Schäfer <ts@aspectis.net>
  */
@@ -17,7 +16,6 @@ $user = $account->isLoggedIn();
 
 <div class="resolverlinks-print">
   <?php // finc-specific change #8447 - CK ?>
-  <?php $noResolverContent = true; ?>
   <?php if (!empty($this->messages) && $user): ?>
     <div class="ezb-message">
       <?php foreach ($this->messages as $link): ?>
@@ -26,9 +24,11 @@ $user = $account->isLoggedIn();
     </div>
   <?php else: ?>
     <?php
-    $url = '<a href="'.$this->url('myresearch-account').'?auth_method='.$account->getAuthMethod().'" target="_blank" data-lightbox>'.$this->transEsc('getit_text_3_2').'</a>';
+    $url = '<a'
+      . ' href="' . $this->url('myresearch-account') . '?auth_method=' . $account->getAuthMethod() . '"'
+      . ' data-lightbox'
+      . '>' . $this->transEsc('getit_text_3_2') . '</a>';
     ?>
-    <?=sprintf($this->translate('getit_text_10'), $url)?>
     <?php $script = <<<JS
 $(document).ready(function() {
   // Use to register new lightbox window pda form
@@ -40,7 +40,6 @@ JS;
   <?php endif; ?>
 
   <?php if (!empty($this->print)): ?>
-    <?php $noResolverContent = false; ?>
     <div class="openurls">
       <ul class="unstyled-list">
         <?php foreach ($this->print as $link): ?>
@@ -67,13 +66,21 @@ JS;
         <?php endforeach; ?>
       </ul>
     </div>
+  <?php else: ?>
+    <?php // finc-specific change #9274 - CK ?>
+    <?php // no content to show, so tell the user ?>
+    <span id="noResolverContentMessage" class="hidden">
+      <?php // TODO: Check if resolver names should always be uppercased ?>
+      <?=($this->resolver ? strtoupper($this->resolver) . ': ' : '') . $this->translate('no_resolver_links')?>
+    </span>
+    <script>
+      if ($('#urlsHideable').length) {
+        $('#urlsHideable').show();
+      } else {
+        $('#noResolverContentMessage').show();
+      }
+    </script>
+    <?php // finc-specific change #9274 - END ?>
   <?php endif; ?>
-
-  <?php // finc-specific change #9274 - CK ?>
-  <?php if ($noResolverContent): // no content to show, so tell the user ?>
-    <span id="noResolverContentMessage" class="hidden"><?=$this->resolver . ': ' . $this->translate('no_resolver_links')?></span>
-    <script>if ($('#urlsHideable').length) { $('#urlsHideable').show(); } else { $('#noResolverContentMessage').show(); }</script>
-  <?php endif; ?>
-  <?php // finc-specific change #9274 - END ?>
 </div>
 <!-- fid-bbi: ajax - resolverlinks-print - END -->

themes/fid_bbi/templates/ajax/resolverLinks.phtml

 <!-- fid_bbi: ajax - resolverlinks -->
-<?php /* this is an exact copy of the finc version to bypass the fid-specific template */?>
+<?php
+/**
+ * This is a modified copy of themes/bbi/templates/ajax/resolverLinks-print
+ * Changes for FID BBI:
+ * - use external link view helper
+ * - use if/else instead of variable
+ *
+ * @author Tobias Schäfer <ts@aspectis.net>
+ */
+?>
+
+<?php
+// Remove empty and duplicate links (determined by href, not title, which can be duplicate)
+$hrefs = [];
+$links = array_filter($this->electronic ?? [], function ($link) use (&$hrefs) {
+  if (!empty($link['href']) && !in_array($link['href'], $hrefs)) {
+    $hrefs[] = $link['href'];
+    return true;
+  }
+});
+?>
+
 <div class="resolver-links">
-  <?php // finc-specific change #8447 - CK ?>
-  <?php $noResolverContent = true; ?>
-  <?php if (!empty($this->electronic)): ?>
-    <?php $noResolverContent = false; ?>
-    <?php // finc-specific change #8447 - END ?>
+  <?php if ($links): ?>
     <div class="openurls">
       <ul class="unstyled-list">
-        <?php foreach ($this->electronic as $link): ?>
+        <?php $titles = []; ?>
+        <?php foreach ($links as $link): ?>
           <li>
-            <?php if (!empty($link['href'])): ?>
-              <?php
-              $linkText = $this->icon('external-link')
-                . $this->transEsc($this->escapeHtml($link['title'] ?? 'unknown'));
-              $linkAttr = [
-                'class' => 'link-with-icon' . (!empty($link['access']) ? ' access-' . $link['access'] : ''),
-                'title' => $this->escapeHtmlAttr($link['service_type'] ?? ''),
-              ];
-              ?>
-              <?=$this->externalLink($this->escapeHtmlAttr($link['href']), $linkText, $linkAttr)?>
-              <br>
+            <?php
+            $title = $this->escapeHtml($link['title'] ?? 'unknown');
+            $titles[$title] = ($titles[$title] ?? 0) + 1;
+            $linkText = $this->icon('external-link') . $this->transEsc($title);
+
+            // Add counter to duplicate link titles, starting with "(2)"
+            if ($titles[$title] > 1) {
+              $linkText .= " ($titles[$title])";
+            }
 
-              <?php // finc-specific change #5334 - CK ?>
-              <small>
-                <?=isset($link['coverage']) ? $this->escapeHtml($link['coverage']) : ''?>
-                <?php if (isset($link['coverageHref'])): ?>
-                  <?php $labelHtml = $this->icon('external-link') . $this->translate('Readme'); ?>
-                  <?=$this->externalLink($link['coverageHref'], $labelHtml)?>
-                <?php endif; ?>
-              </small>
-              <?php // finc-specific change #5334 - END ?>
-            <?php else: ?>
-              <?=isset($link['title']) ? $this->escapeHtml($link['title']) : ''?>
+            $linkAttr = [
+              'class' => 'link-with-icon' . (!empty($link['access']) ? ' access-' . $link['access'] : ''),
+              'title' => $this->escapeHtmlAttr($link['service_type'] ?? ''),
+            ];
+            ?>
+            <?=$this->externalLink($this->escapeHtmlAttr($link['href']), $linkText, $linkAttr)?>
+            <br>
+
+            <?php // finc-specific change #5334 - CK ?>
+            <small>
               <?=isset($link['coverage']) ? $this->escapeHtml($link['coverage']) : ''?>
-            <?php endif; ?>
+              <?php if (isset($link['coverageHref'])): ?>
+                <?php $labelHtml = $this->icon('external-link') . $this->translate('Readme'); ?>
+                <?=$this->externalLink($link['coverageHref'], $labelHtml)?>
+              <?php endif; ?>
+            </small>
+            <?php // finc-specific change #5334 - END ?>
           </li>
         <?php endforeach; ?>
       </ul>
     </div>
-  <?php endif; ?>
-
-  <?php // finc-specific change #9274 - CK ?>
-  <?php if ($noResolverContent): // no content to show, so tell the user ?>
+  <?php else: ?>
+    <?php // finc-specific change #9274 - CK ?>
+    <?php // no content to show, so tell the user ?>
     <span id="noResolverContentMessage" class="hidden">
-      <?=$this->resolver . ': ' . $this->translate('no_resolver_links')?>
+      <?php // TODO: Check if resolver names should always be uppercased ?>
+      <?=($this->resolver ? strtoupper($this->resolver) . ': ' : '') . $this->translate('no_resolver_links')?>
     </span>
     <script>
       if ($('#urlsHideable').length) {
@@ -53,7 +73,7 @@
         $('#noResolverContentMessage').show();
       }
     </script>
+    <?php // finc-specific change #9274 - END ?>
   <?php endif; ?>
-  <?php // finc-specific change #9274 - END ?>
 </div>
 <!-- fid_bbi: ajax - resolverlinks - END -->

themes/fid_bbi/templates/get-it-box/links.phtml

@@ -3,10 +3,10 @@
   <div id="side-collapse-links-data">
     <?php if (!empty($urls)): ?>
       <ul class="unstyled-list">
-        <?php foreach ($urls as $current): ?>
+        <?php foreach ($urls as $url): ?>
           <li>
-            <?php $linkText = $this->icon('external-link') . $this->escapeHtml($current['desc']); ?>
-            <?=$this->externalLink($this->escapeHtmlAttr($current['url']), $linkText, ['class' => 'link-with-icon'])?>
+            <?php $linkText = $this->icon('external-link') . $this->escapeHtml($url['desc']); ?>
+            <?=$this->externalLink($this->escapeHtmlAttr($url['url']), $linkText, ['class' => 'link-with-icon'])?>
           </li>
         <?php endforeach; ?>
       </ul>
@@ -16,19 +16,29 @@
     <?php if ($openUrlActive): ?>
       <?php if (empty($urls)): ?>
         <?php
-        $fallbackUrls = $this->driver->tryMethod('getURLs');
+        $fallbackUrls = $this->driver->tryMethod('getURLs') ?? [];
+        $doiUrls = array_filter($fallbackUrls, function ($url) {
+          // TODO: This could lead to false positives.
+          return strpos($url['url'], 'doi') !== false;
+        });
         $doiName = $this->driver->tryMethod('getCleanDOI');
         ?>
-        <?php if (!empty($fallbackUrls)): ?>
-          <p id="urlsHideable" style="display: none">
-            <?php foreach ($fallbackUrls as $current): ?>
-              <?php if (preg_match_all('/doi/', $current['url'])): ?>
-                <a href="<?=$this->escapeHtmlAttr($this->proxyUrl($current['url']))?>">
-                  DOI: <?=!empty($doiName) ? $doiName : $current['url'] ?>
-                </a>
-              <?php endif; ?>
+        <?php if ($doiUrls): ?>
+          <ul id="urlsHideable" class="unstyled-list" style="display: none">
+            <?php foreach ($doiUrls as $url): ?>
+              <li>
+                <?php $linkLabel = 'DOI: ' . (!empty($doiName) ? $doiName : $url['url']); ?>
+                <?=$this->externalLink($this->escapeHtmlAttr($this->proxyUrl($url['url'])), $linkLabel)?>
+              </li>
             <?php endforeach; ?>
-          </p>
+          </ul>
+        <?php elseif ($fallbackUrls): ?>
+          <?php
+          // TODO: There is a script displaying a message when #urlsHideable is
+          // missing, but at least it's not a <p> or <ul> anymore, which
+          // added margin.
+          ?>
+          <div id="urlsHideable"></div>
         <?php endif; ?>
       <?php endif; ?>