From 8134253be87f82d24a03cc75ff59114465a90020 Mon Sep 17 00:00:00 2001 From: Demian Katz Date: Thu, 19 Dec 2024 09:00:43 -0500 Subject: [PATCH] Refactor link rendering to template. --- .../AjaxHandler/IdentifierLinksLookup.php | 30 ++-- .../AjaxHandler/IdentifierLinksLookupTest.php | 137 ++++++++++-------- themes/bootstrap3/js/identifierLinks.js | 32 +--- .../templates/ajax/identifierLinks.phtml | 11 ++ themes/bootstrap5/js/identifierLinks.js | 32 +--- .../templates/ajax/identifierLinks.phtml | 11 ++ 6 files changed, 120 insertions(+), 133 deletions(-) create mode 100644 themes/bootstrap3/templates/ajax/identifierLinks.phtml create mode 100644 themes/bootstrap5/templates/ajax/identifierLinks.phtml diff --git a/module/VuFind/src/VuFind/AjaxHandler/IdentifierLinksLookup.php b/module/VuFind/src/VuFind/AjaxHandler/IdentifierLinksLookup.php index 75913678a14..6186bb59f3e 100644 --- a/module/VuFind/src/VuFind/AjaxHandler/IdentifierLinksLookup.php +++ b/module/VuFind/src/VuFind/AjaxHandler/IdentifierLinksLookup.php @@ -109,36 +109,46 @@ public function __construct( */ public function handleRequest(Params $params) { - $response = []; + $gatheredData = []; $ids = json_decode($params->getController()->getRequest()->getContent(), true); foreach ($this->resolvers as $resolver) { if ($this->pluginManager->has($resolver)) { $next = $this->pluginManager->get($resolver)->getLinks($ids); $next = $this->processIconLinks($next); foreach ($next as $key => $data) { - foreach ($data as &$current) { - $current['newWindow'] = $this->openInNewWindow; - } - unset($current); - if (!isset($response[$key])) { - $response[$key] = $data; + if (!isset($gatheredData[$key])) { + $gatheredData[$key] = $data; } elseif ($this->multiMode == 'merge') { - $response[$key] = array_merge($response[$key], $data); + $gatheredData[$key] = array_merge($gatheredData[$key], $data); } } // If all keys have been found and we're not in merge mode, we // can short circuit out of here. if ( $this->multiMode !== 'merge' - && count(array_diff(array_keys($ids), array_keys($response))) == 0 + && count(array_diff(array_keys($ids), array_keys($gatheredData))) == 0 ) { break; } } } + $response = array_map([$this, 'renderResponseChunk'], $gatheredData); return $this->formatResponse($response); } + /** + * Render the links for a single record. + * + * @param array $data Data to render + * + * @return string + */ + protected function renderResponseChunk(array $data): string + { + $newWindow = $this->openInNewWindow; + return $this->viewRenderer->render('ajax/identifierLinks.phtml', compact('data', 'newWindow')); + } + /** * Proxify external icon links and render local icons * @@ -164,7 +174,7 @@ protected function processIconLinks(array $data): array ); } if (!empty($link['localIcon'])) { - $link['localIcon'] = $iconHelper($link['localIcon']); + $link['localIcon'] = $iconHelper($link['localIcon'], 'icon-link__icon'); } } unset($link); diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/AjaxHandler/IdentifierLinksLookupTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/AjaxHandler/IdentifierLinksLookupTest.php index d4118996025..494ee9cf616 100644 --- a/module/VuFind/tests/unit-tests/src/VuFindTest/AjaxHandler/IdentifierLinksLookupTest.php +++ b/module/VuFind/tests/unit-tests/src/VuFindTest/AjaxHandler/IdentifierLinksLookupTest.php @@ -35,6 +35,8 @@ use VuFind\IdentifierLinker\IdentifierLinkerInterface; use VuFind\IdentifierLinker\PluginManager; +use function func_get_args; + /** * IdentifierLinksLookup test class. * @@ -144,6 +146,15 @@ function ($plugin) use ($plugins) { return $plugins[$plugin] ?? null; } ); + // JSON encode parameters to the render method so that it returns a string + // that we can make assertions about in our tests. + $mockRenderer->expects($this->any()) + ->method('render') + ->willReturnCallback( + function () { + return json_encode(func_get_args()); + } + ); $this->container->set('ViewRenderer', $mockRenderer); @@ -215,18 +226,19 @@ public function testSingleLookup( ); // Test the handler: + $expectedTemplate = 'ajax/identifierLinks.phtml'; + $expectedLinks = [ + [ + 'link' => 'http://baz', + 'label' => 'baz', + 'icon' => $remoteIcon, + 'localIcon' => '(local-icon)', + ], + ]; $this->assertEquals( [ [ - 0 => [ - [ - 'link' => 'http://baz', - 'label' => 'baz', - 'newWindow' => $newWindow, - 'icon' => $remoteIcon, - 'localIcon' => '(local-icon)', - ], - ], + 0 => json_encode([$expectedTemplate, ['data' => $expectedLinks, 'newWindow' => $newWindow]]), ], ], $this->getHandlerResults() @@ -252,18 +264,19 @@ public function testFirstDefaultLookup() ); // Test the handler: + $expectedTemplate = 'ajax/identifierLinks.phtml'; + $expectedLinks = [ + [ + 'link' => 'http://baz', + 'label' => 'baz', + 'icon' => 'remote-icon', + 'localIcon' => '(local-icon)', + ], + ]; $this->assertEquals( [ [ - 0 => [ - [ - 'link' => 'http://baz', - 'label' => 'baz', - 'newWindow' => false, - 'icon' => 'remote-icon', - 'localIcon' => '(local-icon)', - ], - ], + 0 => json_encode([$expectedTemplate, ['data' => $expectedLinks, 'newWindow' => false]]), ], ], $this->getHandlerResults() @@ -291,18 +304,19 @@ public function testFirstExplicitLookup() ); // Test the handler: + $expectedTemplate = 'ajax/identifierLinks.phtml'; + $expectedLinks = [ + [ + 'link' => 'http://baz', + 'label' => 'baz', + 'icon' => 'remote-icon', + 'localIcon' => '(local-icon)', + ], + ]; $this->assertEquals( [ [ - 0 => [ - [ - 'link' => 'http://baz', - 'label' => 'baz', - 'newWindow' => false, - 'icon' => 'remote-icon', - 'localIcon' => '(local-icon)', - ], - ], + 0 => json_encode([$expectedTemplate, ['data' => $expectedLinks, 'newWindow' => false]]), ], ], $this->getHandlerResults() @@ -335,27 +349,28 @@ public function testFirstExplicitLookupMultipleDOIs() ); // Test the handler: + $expectedTemplate = 'ajax/identifierLinks.phtml'; + $expectedLinks0 = [ + [ + 'link' => 'http://baz', + 'label' => 'baz', + 'icon' => 'remote-icon', + 'localIcon' => '(local-icon)', + ], + ]; + $expectedLinks1 = [ + [ + 'link' => 'http://baz2', + 'label' => 'baz2', + 'icon' => 'remote-icon', + 'localIcon' => '(local-icon)', + ], + ]; $this->assertEquals( [ [ - 0 => [ - [ - 'link' => 'http://baz', - 'label' => 'baz', - 'newWindow' => false, - 'icon' => 'remote-icon', - 'localIcon' => '(local-icon)', - ], - ], - 1 => [ - [ - 'link' => 'http://baz2', - 'label' => 'baz2', - 'newWindow' => false, - 'icon' => 'remote-icon', - 'localIcon' => '(local-icon)', - ], - ], + 0 => json_encode([$expectedTemplate, ['data' => $expectedLinks0, 'newWindow' => false]]), + 1 => json_encode([$expectedTemplate, ['data' => $expectedLinks1, 'newWindow' => false]]), ], ], $this->getHandlerResults($request) @@ -382,25 +397,25 @@ public function testMergeLookup() ] ); // Test the handler: + $expectedTemplate = 'ajax/identifierLinks.phtml'; + $expectedLinks = [ + [ + 'link' => 'http://baz', + 'label' => 'baz', + 'icon' => 'remote-icon', + 'localIcon' => '(local-icon)', + ], + [ + 'link' => 'http://baz2', + 'label' => 'baz2', + 'icon' => 'remote-icon', + 'localIcon' => '(local-icon)', + ], + ]; $this->assertEquals( [ [ - 0 => [ - [ - 'link' => 'http://baz', - 'label' => 'baz', - 'newWindow' => false, - 'icon' => 'remote-icon', - 'localIcon' => '(local-icon)', - ], - [ - 'link' => 'http://baz2', - 'label' => 'baz2', - 'newWindow' => false, - 'icon' => 'remote-icon', - 'localIcon' => '(local-icon)', - ], - ], + 0 => json_encode([$expectedTemplate, ['data' => $expectedLinks, 'newWindow' => false]]), ], ], $this->getHandlerResults() diff --git a/themes/bootstrap3/js/identifierLinks.js b/themes/bootstrap3/js/identifierLinks.js index 41ce095aee2..dd2f45c4cde 100644 --- a/themes/bootstrap3/js/identifierLinks.js +++ b/themes/bootstrap3/js/identifierLinks.js @@ -33,37 +33,7 @@ VuFind.register('identifierLinks', function identifierLinks() { var currentInstance = identifierEl.dataset.instance; rawResponse.json().then(response => { if ("undefined" !== typeof response.data[currentInstance]) { - identifierEl.textContent = ""; - for (var i = 0; i < response.data[currentInstance].length; i++) { - var newLink = document.createElement('a'); - newLink.classList.add('icon-link'); - newLink.setAttribute('href', response.data[currentInstance][i].link); - if (typeof response.data[currentInstance][i].icon !== 'undefined') { - var icon = document.createElement('img'); - icon.setAttribute('src', response.data[currentInstance][i].icon); - icon.classList.add("identifier-link-icon"); - icon.classList.add("icon-link__icon"); - newLink.appendChild(icon); - } else if (typeof response.data[currentInstance][i].localIcon !== 'undefined') { - var localIconWrapper = document.createElement('span'); - VuFind.setInnerHtml(localIconWrapper, response.data[currentInstance][i].localIcon); - var localIcon = localIconWrapper.firstChild; - if (localIcon) { - localIcon.classList.add('icon-link__icon'); - newLink.appendChild(localIcon); - } - } - var newSpan = document.createElement('span'); - newSpan.setAttribute("rel", "noreferrer"); - if (response.data[currentInstance][i].newWindow) { - newSpan.setAttribute("target", '_blank'); - } - newSpan.classList.add('icon-link__label'); - newSpan.appendChild(document.createTextNode(response.data[currentInstance][i].label)); - newLink.appendChild(newSpan); - identifierEl.appendChild(newLink); - identifierEl.appendChild(document.createElement('br')); - } + VuFind.setInnerHtml(identifierEl, response.data[currentInstance]); } }); }); diff --git a/themes/bootstrap3/templates/ajax/identifierLinks.phtml b/themes/bootstrap3/templates/ajax/identifierLinks.phtml new file mode 100644 index 00000000000..64663a7a9da --- /dev/null +++ b/themes/bootstrap3/templates/ajax/identifierLinks.phtml @@ -0,0 +1,11 @@ + 'icon-link', 'rel' => 'noreferrer'] + (($newWindow ?? false) ? ['target' => '_blank'] : []); ?> + + htmlAttributes($baseLinkAttrs + ['href' => $link['link']])?>> + + htmlAttributes(['src' => $link['icon'], 'class' => 'identifier-link-icon icon-link__icon'])?> + + + + htmlAttributes(['class' => 'icon-link__label'])?>>escapeHtml($link['label'])?> +
+ \ No newline at end of file diff --git a/themes/bootstrap5/js/identifierLinks.js b/themes/bootstrap5/js/identifierLinks.js index 41ce095aee2..dd2f45c4cde 100644 --- a/themes/bootstrap5/js/identifierLinks.js +++ b/themes/bootstrap5/js/identifierLinks.js @@ -33,37 +33,7 @@ VuFind.register('identifierLinks', function identifierLinks() { var currentInstance = identifierEl.dataset.instance; rawResponse.json().then(response => { if ("undefined" !== typeof response.data[currentInstance]) { - identifierEl.textContent = ""; - for (var i = 0; i < response.data[currentInstance].length; i++) { - var newLink = document.createElement('a'); - newLink.classList.add('icon-link'); - newLink.setAttribute('href', response.data[currentInstance][i].link); - if (typeof response.data[currentInstance][i].icon !== 'undefined') { - var icon = document.createElement('img'); - icon.setAttribute('src', response.data[currentInstance][i].icon); - icon.classList.add("identifier-link-icon"); - icon.classList.add("icon-link__icon"); - newLink.appendChild(icon); - } else if (typeof response.data[currentInstance][i].localIcon !== 'undefined') { - var localIconWrapper = document.createElement('span'); - VuFind.setInnerHtml(localIconWrapper, response.data[currentInstance][i].localIcon); - var localIcon = localIconWrapper.firstChild; - if (localIcon) { - localIcon.classList.add('icon-link__icon'); - newLink.appendChild(localIcon); - } - } - var newSpan = document.createElement('span'); - newSpan.setAttribute("rel", "noreferrer"); - if (response.data[currentInstance][i].newWindow) { - newSpan.setAttribute("target", '_blank'); - } - newSpan.classList.add('icon-link__label'); - newSpan.appendChild(document.createTextNode(response.data[currentInstance][i].label)); - newLink.appendChild(newSpan); - identifierEl.appendChild(newLink); - identifierEl.appendChild(document.createElement('br')); - } + VuFind.setInnerHtml(identifierEl, response.data[currentInstance]); } }); }); diff --git a/themes/bootstrap5/templates/ajax/identifierLinks.phtml b/themes/bootstrap5/templates/ajax/identifierLinks.phtml new file mode 100644 index 00000000000..64663a7a9da --- /dev/null +++ b/themes/bootstrap5/templates/ajax/identifierLinks.phtml @@ -0,0 +1,11 @@ + 'icon-link', 'rel' => 'noreferrer'] + (($newWindow ?? false) ? ['target' => '_blank'] : []); ?> + + htmlAttributes($baseLinkAttrs + ['href' => $link['link']])?>> + + htmlAttributes(['src' => $link['icon'], 'class' => 'identifier-link-icon icon-link__icon'])?> + + + + htmlAttributes(['class' => 'icon-link__label'])?>>escapeHtml($link['label'])?> +
+ \ No newline at end of file