Skip to content

Update plugins.xml - #514

Open
stefesciberras wants to merge 1 commit into
pkp:mainfrom
stefesciberras:add-reviewer-comments-plugin
Open

Update plugins.xml#514
stefesciberras wants to merge 1 commit into
pkp:mainfrom
stefesciberras:add-reviewer-comments-plugin

Conversation

@stefesciberras

Copy link
Copy Markdown

Add the Reviewer Comments generic plugin for OJS 3.4.

Repository:
https://github.com/stefesciberras/ojs-plugin-reviewer-comments

Release:
1.1.0.0

Add the Reviewer Comments generic plugin for OJS 3.4.

Repository:
https://github.com/stefesciberras/ojs-plugin-reviewer-comments

Release:
1.1.0.0
@bozana

bozana commented Aug 13, 2026

Copy link
Copy Markdown

Hi @stefesciberras,

I am wondering if there is maybe a typo in the email address in this PR: stepehn.sciberras@mmsjournals.org -- should it maybe be stephen instead of stepehn?
Could you please also remove the empty lines between elements -- for the file to stay consistent.
You can add the element <certification type="reviewed"/> as elsewhere in plugins.xml.
The JS is added to every template. I think it would be better to narrow it to the one it needs to be added to, something like this:

if (!$templateMgr || $templatePath !== 'decision/record.tpl') {
      return false;
}

Would you mind changing this?

Not a merge-stopper, but maybe you would like to also consider the following:

  • HANDLER_CLASS — the plugin registers its page handler via define('HANDLER_CLASS', 'APP\\plugins\\generic\\reviewerComments\\ReviewerCommentsHandler'); in the LoadHandler hook callback. This is marked deprecated in OJS 3.4's core and has been removed entirely in OJS 3.5. It still works correctly on the plugin's target (3.4.x), but would need change at least for 3.5 on.
    As an example, core's own bundled staticPages plugin shows the non-deprecated way: grab the hook's own by-reference $handler parameter and assign the instance directly ($handler = new StaticPagesHandler($this, $staticPage);) instead of using the HANDLER_CLASS constant.
  • English-only button matching — the frontend JS locates the "Insert Content" button to attach next to via a hardcoded regex, /insert content/i. On any non-English backend locale (pt_BR, es, etc.), that button's label won't match, so the plugin's control silently never appears — a real functional gap for a platform where non-English admin UIs are common. Maybe to use tiny.settings.toolbar.includes('pkpInsert') instead -- the JS already locates the active editor instance via findTinyMce()?

Thanks!

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.

2 participants