Skip to content

Conversation

@nongenti
Copy link
Contributor

@asmecher asmecher changed the title Add citationStyleLanguagePlugin support pkp/pkp-lib/issues/6201 Add citationStyleLanguagePlugin support May 17, 2022

{* How to cite *}
{if $citation}
{include file="../plugins/generic/citationStyleLanguage/templates/citationblock.tpl"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be possible to code the plugin to augment OMP without needing to be called from within the OMP code. I'd suggest following the pattern used by the OJS citationStyleLanguage plugin in this regard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, @NateWr told me to do it at this way, see pkp/citationStyleLanguage#84 (comment). Idea was to change it also in OJS. Then there is only one template, used at three places and it's in the plugin it belongs to. But if you want, then I change it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, the default theme in OJS includes the full template in article_details.tpl. However, the JavaScript that makes it work (articleCitation.js) and the data assigned to $templateMgr all live in the citation style language plugin.

I'm not too bothered either way, but it seemed sensible to bring it all together in one place and reduce the code duplication.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for sending you in two directions, @nongenti 🥴

The idea is that plugins should be self-contained, i.e. they shouldn't require any changes to the core codebase. This change refers directly to a plugin template from the core codebase, which I'd like to avoid. We're privileged in that we can ship plugins with the software and entangle the core codebase and plugin code as much as we like, but 1) there's not much point in coding it as a plugin if we're integrating part of it anyway, and 2) other (third party) plugins don't have the same privilege and it would be better to set down patterns that the community can use.

We don't have a 100% clear policy on "plugins that alter plugins" -- e.g. a theme plugin that wants to alter the markup generated in this plugin. In my opinion, we should try to avoid the situation because it'll lead to complicated dependencies -- the author of a theme plugin will have to know the development details of the other plugins it interacts with, in addition to OJS.

The "entanglement" between citationStyleLanguage and OJS already existed before this PR, but I like @NateWr's idea of moving the template into this plugin so that less code is duplicated. The thing I'd like to fix is the way the OJS/OMP code calls on it. Rather than using {include} with a path into the plugin's codebase, use the existing Templates::Article::Details hook:

diff --git a/CitationStyleLanguagePlugin.inc.php b/CitationStyleLanguagePlugin.inc.php
index e72ff9b..f71bf89 100644
--- a/CitationStyleLanguagePlugin.inc.php
+++ b/CitationStyleLanguagePlugin.inc.php
@@ -72,10 +72,20 @@ class CitationStyleLanguagePlugin extends GenericPlugin
             HookRegistry::register('ArticleHandler::view', [$this, 'getArticleTemplateData']);
             HookRegistry::register('PreprintHandler::view', array($this, 'getPreprintTemplateData'));
             HookRegistry::register('LoadHandler', [$this, 'setPageHandler']);
+            HookRegistry::register('Templates::Article::Details', [$this, 'addCitationMarkup']);
         }
         return $success;
     }
 
+    public function addCitationMarkup($hookName, $args) {
+        $params =& $args[0];
+        $smarty =& $args[1];
+        $output =& $args[2];
+
+        $output .= /* ...code to fetch the contents of the citation template goes here... */
+        return false;
+    }
+
     /**
      * Get list of citation styles available
      *

...calling on the citation block template that you've now moved into this plugin. (The equivalent OMP hook is Templates::Catalog::Book::Details.)

As for how themes etc. can alter the presentation of citations, as I say, I'm hoping we can avoid substantive changes -- but good markup classnaming will e.g. allow plugins to style citations, which I'm hoping is enough.

(One feature we're missing in either implementation is the ability to reorganize the sidebar content in the article view. At some point we might want to reconcile our article sidebar implementation with our website sidebar blocks implementation, but let's not get into that now.)

@NateWr, I trust your instincts on theme development/maintenance. Does that work OK for you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree using the hook is best in general. But it will change the order of content in the side area, and I do think people will be unhappy about that. (Maybe it's ok for a major release.) In the past, we've included things we consider "core" plugins right into the template (ORCID, DOI, How to Cite).

The long-term solution is to break the article sidebar components into "blocks" that can be re-ordered. But we've had that on the todo list for years and never got round to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll change it to using the hook.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants