Fix Channel Popovers (#3607)

module/VuFind/src/VuFindTest/Integration/MinkTestCase.php

@@ -37,6 +37,7 @@
 use VuFind\Config\PathResolver;
 use VuFind\Config\Writer as ConfigWriter;
 
+use function call_user_func;
 use function floatval;
 use function in_array;
 use function intval;
@@ -562,6 +563,87 @@ protected function findCssAndSetValue(
         throw new \Exception('Failed to set value after ' . $retries . ' attempts.');
     }
 
+    /**
+     * Get text of an element selected via CSS; retry if it fails due to DOM change.
+     *
+     * @param Element $page     Page element
+     * @param string  $selector CSS selector
+     * @param int     $timeout  Wait timeout for CSS selection (in ms)
+     * @param int     $index    Index of the element (0-based)
+     * @param int     $retries  Retry count for set loop
+     *
+     * @return string
+     */
+    protected function findCssAndGetText(
+        Element $page,
+        $selector,
+        $timeout = null,
+        $index = 0,
+        $retries = 6
+    ) {
+        return $this->findCssAndCallMethod($page, $selector, 'getText', $timeout, $index, $retries);
+    }
+
+    /**
+     * Get text of an element selected via CSS; retry if it fails due to DOM change.
+     *
+     * @param Element $page     Page element
+     * @param string  $selector CSS selector
+     * @param int     $timeout  Wait timeout for CSS selection (in ms)
+     * @param int     $index    Index of the element (0-based)
+     * @param int     $retries  Retry count for set loop
+     *
+     * @return string
+     */
+    protected function findCssAndGetHtml(
+        Element $page,
+        $selector,
+        $timeout = null,
+        $index = 0,
+        $retries = 6
+    ) {
+        return $this->findCssAndCallMethod($page, $selector, 'getHtml', $timeout, $index, $retries);
+    }
+
+    /**
+     * Return value of a method of an element selected via CSS; retry if it fails due to DOM change.
+     *
+     * @param Element  $page     Page element
+     * @param string   $selector CSS selector
+     * @param callable $method   Method to call
+     * @param int      $timeout  Wait timeout for CSS selection (in ms)
+     * @param int      $index    Index of the element (0-based)
+     * @param int      $retries  Retry count for set loop
+     *
+     * @return string
+     */
+    protected function findCssAndCallMethod(
+        Element $page,
+        $selector,
+        $method,
+        $timeout = null,
+        $index = 0,
+        $retries = 6,
+    ) {
+        $timeout ??= $this->getDefaultTimeout();
+
+        for ($i = 1; $i <= $retries; $i++) {
+            try {
+                $element = $this->findCss($page, $selector, $timeout, $index);
+                return call_user_func([$element, $method]);
+            } catch (\Exception $e) {
+                $this->logWarning(
+                    'RETRY findCssAndGetText after exception in ' . $this->getTestName()
+                    . " (try $i): " . (string)$e
+                );
+            }
+
+            $this->snooze();
+        }
+
+        throw new \Exception('Failed to get text after ' . $retries . ' attempts.');
+    }
+
     /**
      * Retrieve a link and assert that it exists before returning it.
      *

module/VuFind/tests/integration-tests/src/VuFindTest/Mink/ChannelsTest.php

@@ -1,11 +1,11 @@
 <?php
 
 /**
- * Mink cart test class.
+ * Mink channels test class.
  *
  * PHP version 8
  *
- * Copyright (C) Villanova University 2011.
+ * Copyright (C) Villanova University 2011-2024.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2,
@@ -32,7 +32,7 @@
 use Behat\Mink\Element\Element;
 
 /**
- * Mink cart test class.
+ * Mink channels test class.
  *
  * @category VuFind
  * @package  Tests
@@ -48,7 +48,7 @@ class ChannelsTest extends \VuFindTest\Integration\MinkTestCase
      *
      * @return Element
      */
-    protected function getChannelsPage()
+    protected function getChannelsPage(): Element
     {
         $session = $this->getMinkSession();
         $path = '/Channels/Search?lookfor=building%3A%22weird_ids.mrc%22';
@@ -61,7 +61,7 @@ protected function getChannelsPage()
      *
      * @return void
      */
-    public function testBasic()
+    public function testBasic(): void
     {
         $page = $this->getChannelsPage();
         // Channels are here
@@ -81,7 +81,7 @@ public function testBasic()
      *
      * @return void
      */
-    public function testAddChannels()
+    public function testAddChannels(): void
     {
         $page = $this->getChannelsPage();
         $channel = $this->findCss($page, 'div.channel-wrapper');
@@ -102,7 +102,7 @@ public function testAddChannels()
      *
      * @return void
      */
-    public function testSwitchToSearch()
+    public function testSwitchToSearch(): void
     {
         $page = $this->getChannelsPage();
         $channel = $this->findCss($page, 'div.channel-wrapper');
@@ -125,4 +125,38 @@ public function testSwitchToSearch()
             $this->findCss($page, '.filters .filter-value')->getText()
         );
     }
+
+    /**
+     * Test popover behavior
+     *
+     * @return void
+     */
+    public function testPopovers(): void
+    {
+        $page = $this->getChannelsPage();
+        // Click a record to open the popover:
+        $this->clickCss($page, '.channel-record[data-record-id="hashes#coming@ya"]');
+        $popoverContents = $this->findCssAndGetText($page, '.popover');
+        // The popover should contain an appropriate title and metadata:
+        $this->assertStringContainsString('Octothorpes: Why not?', $popoverContents);
+        $this->assertStringContainsString('Physical Description', $popoverContents);
+        // Click a different record:
+        $this->clickCss($page, '.channel-record[data-record-id="dollar$ign/slashcombo"]');
+        $popoverContents2 = $this->findCssAndGetText($page, '.popover');
+        // The popover should contain an appropriate title and metadata:
+        $this->assertStringContainsString('Of Money and Slashes', $popoverContents2);
+        $this->assertStringContainsString('Physical Description', $popoverContents2);
+        // Click outside of channels to move the focus away:
+        $this->clickCss($page, 'li.active');
+        // Now click back to the original record; the popover should contain the same contents.
+        $this->clickCss($page, '.channel-record[data-record-id="hashes#coming@ya"]');
+        $popoverContents3 = $this->findCssAndGetText($page, '.popover');
+        $this->assertEquals($popoverContents, $popoverContents3);
+        // Finally, click through to the record page.
+        $link = $this->findCss($page, '.popover a', null, 1);
+        $this->assertEquals('View Record', $link->getText());
+        $link->click();
+        $this->waitForPageLoad($page);
+        $this->assertEquals('Octothorpes: Why not?', $this->findCssAndGetText($page, 'h1'));
+    }
 }

themes/bootstrap3/js/channels.js

@@ -102,37 +102,58 @@ VuFind.register('channels', function Channels() {
     $(op).on('swipe', function channelDrag() {
       switchPopover(false);
     });
-    $(op).find('.channel-record').off("click").on("click", function channelRecord(event) {
-      var record = $(event.delegateTarget);
-      if (!record.data("popover-loaded")) {
-        record.popover({
-          content: VuFind.translate('loading_ellipsis'),
-          html: true,
-          placement: 'bottom',
-          trigger: 'focus',
-          container: '#' + record.closest('.channel').attr('id')
-        });
+
+    let recordContent = {};
+    function showChannelRecord(event) {
+      const record = $(event.delegateTarget);
+
+      record.popover({
+        content: VuFind.translate('loading_ellipsis'),
+        html: true,
+        placement: 'bottom',
+        trigger: 'focus',
+        container: '#' + record.closest('.channel').attr('id')
+      });
+
+      const recordID = record.data("record-id");
+      if (!(recordID in recordContent)) {
+        const controls = '<div class="btn-group btn-group-justified">'
+          + '<a href="' + VuFind.path + '/Channels/Record?'
+            + 'id=' + encodeURIComponent(record.attr('data-record-id'))
+            + '&source=' + encodeURIComponent(record.attr('data-record-source'))
+          + '" class="btn btn-default">' + VuFind.translate('channel_expand') + '</a>'
+          + '<a href="' + record.attr('href') + '" class="btn btn-default">' + VuFind.translate('View Record') + '</a>'
+          + '</div>';
+
         $.ajax({
           url: VuFind.path + getUrlRoot(record.attr('href')) + '/AjaxTab',
           type: 'POST',
           data: {tab: 'description'}
         })
           .done(function channelPopoverDone(data) {
-            var newContent = '<div class="btn-group btn-group-justified">'
-            + '<a href="' + VuFind.path + '/Channels/Record?'
-              + 'id=' + encodeURIComponent(record.attr('data-record-id'))
-              + '&source=' + encodeURIComponent(record.attr('data-record-source'))
-            + '" class="btn btn-default">' + VuFind.translate('channel_expand') + '</a>'
-            + '<a href="' + record.attr('href') + '" class="btn btn-default">' + VuFind.translate('View Record') + '</a>'
-            + '</div>'
-            + data;
-            redrawPopover(record, newContent);
-            record.data("popover-loaded", true);
+            recordContent[recordID] = controls + data;
+          })
+          .fail(function channelPopoverFail(error) {
+            console.error(error);
+            recordContent[recordID] = controls + VuFind.translate('no_description');
+          })
+          .always(function channelPopoverFinally() {
+            switchPopover(record);
+            redrawPopover(record, recordContent[recordID]);
           });
+      } else {
+        switchPopover(record);
+        redrawPopover(record, recordContent[recordID]);
+        // redraw needs to happen since element is destroyed on hide
       }
-      switchPopover(record);
       return false;
-    });
+    }
+
+    // don't follow links
+    $(op).find('.channel-record').off("click").on("click", () => false);
+    // click and tab
+    $(op).find('.channel-record').off("focus").on("focus", showChannelRecord);
+
     // Channel add buttons
     addLinkButtons(op);
     $('.channel-add-menu[data-group="' + op.dataset.group + '"].hidden')

themes/bootstrap3/templates/layout/js-translations.phtml

@@ -24,6 +24,7 @@ $this->jsTranslations()->addStrings(
       'libphonenumber_tooshortidd' => 'libphonenumber_tooshortidd',
       'loading_ellipsis' => 'loading_ellipsis',
       'more_ellipsis' => 'more_ellipsis',
+      'no_description' => 'no_description',
       'number_thousands_separator' => [
         'number_thousands_separator', null, ',',
       ],