-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pkp/pkp-lib/issues/6201 Adapt to support OMP #79
base: stable-3_3_0
Are you sure you want to change the base?
Changes from 3 commits
b237fd7
1d7f5fe
b756515
5b68c41
9bcab49
ed120eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
.citation_display .value | ||
{ | ||
font-size: 0.75rem; | ||
} | ||
|
||
.citation_display .csl-left-margin | ||
{ | ||
display: none; | ||
} | ||
|
||
.citation_display [aria-hidden="true"] | ||
{ | ||
display: none; | ||
} | ||
|
||
.citation_display .citation_formats | ||
{ | ||
margin-top: 1em; | ||
border: 1px solid rgba(0, 0, 0, 0.4); | ||
border-radius: 2px; | ||
} | ||
|
||
.citation_display .citation_formats_button | ||
{ | ||
position: relative; | ||
background: transparent; | ||
border: none; | ||
border-bottom-left-radius: 0; | ||
border-bottom-right-radius: 0; | ||
box-shadow: none; | ||
padding: 0 1em; | ||
width: 100%; | ||
font-family: "Noto Sans", -apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen-Sans", "Ubuntu", "Cantarell", "Helvetica Neue", sans-serif; | ||
font-weight: 400; | ||
color: #777; | ||
text-align: left; | ||
} | ||
|
||
.citation_display .citation_formats_button:after | ||
{ | ||
display: inline-block; | ||
font: normal normal normal 14px/1 FontAwesome; | ||
font-size: inherit; | ||
text-rendering: auto; | ||
-webkit-font-smoothing: antialiased; | ||
-moz-osx-font-smoothing: grayscale; | ||
content: "\f0d7"; | ||
position: absolute; | ||
top: 50%; | ||
right: 1em; | ||
transform: translate(0, -50%); | ||
} | ||
.citation_display .citation_formats_button[aria-expanded="true"]:after | ||
{ | ||
content: "\f0d8"; | ||
} | ||
|
||
.citation_display .citation_formats_button:focus | ||
{ | ||
background: #ddd; | ||
outline: 0; | ||
} | ||
|
||
.citation_display .citation_formats_styles | ||
{ | ||
margin: 0; | ||
padding: 0; | ||
list-style: none; | ||
} | ||
|
||
.citation_display .citation_formats_styles a | ||
{ | ||
display: block; | ||
padding: 0.5em 1em; | ||
border-bottom: 1px solid #bbb; | ||
text-decoration: none; | ||
} | ||
|
||
.citation_display .citation_formats_styles a:focus | ||
{ | ||
background: #ddd; | ||
outline: 0; | ||
} | ||
|
||
.citation_display .citation_formats_styles li:last-child a | ||
{ | ||
border-bottom: none; | ||
} | ||
|
||
.citation_display .citation_formats_list .label | ||
{ | ||
padding: 1em 1em 0.25em 1em; | ||
} | ||
|
||
.citation_display .citation_formats_styles + .label | ||
{ | ||
border-top: 1px solid #bbb; | ||
} | ||
|
||
.citation_display span | ||
{ | ||
margin-right: 0.5em; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,8 +16,8 @@ | |
import('classes.handler.Handler'); | ||
|
||
class CitationStyleLanguageHandler extends Handler { | ||
/** @var Submission article being requested */ | ||
public $article = null; | ||
/** @var Submission $submission being requested */ | ||
public $submission = null; | ||
|
||
/** @var Publication publication being requested */ | ||
public $publication = null; | ||
|
@@ -42,7 +42,13 @@ public function get($args, $request) { | |
$this->_setupRequest($args, $request); | ||
|
||
$plugin = PluginRegistry::getPlugin('generic', 'citationstylelanguageplugin'); | ||
$citation = $plugin->getCitation($request, $this->article, $this->citationStyle, $this->issue, $this->publication); | ||
if (NULL === $plugin) { | ||
if ($this->returnJson) { | ||
return new JSONMessage(false); | ||
} | ||
exit; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's use the 404 handler instead of exiting early. We can do that with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see that you''re just duplicating the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've changed it. |
||
} | ||
$citation = $plugin->getCitation($request, $this->submission, $this->citationStyle, $this->issue, $this->publication); | ||
|
||
if ($citation === false ) { | ||
if ($this->returnJson) { | ||
|
@@ -69,7 +75,9 @@ public function download($args, $request) { | |
$this->_setupRequest($args, $request); | ||
|
||
$plugin = PluginRegistry::getPlugin('generic', 'citationstylelanguageplugin'); | ||
$plugin->downloadCitation($request, $this->article, $this->citationStyle, $this->issue, $this->publication); | ||
if (NULL !== $plugin) { | ||
$plugin->downloadCitation($request, $this->submission, $this->citationStyle, $this->issue, $this->publication); | ||
} | ||
exit; | ||
} | ||
|
||
|
@@ -94,34 +102,36 @@ public function _setupRequest($args, $request) { | |
|
||
$this->citationStyle = $args[0]; | ||
$this->returnJson = isset($userVars['return']) && $userVars['return'] === 'json'; | ||
$this->article = Services::get('submission')->get($userVars['submissionId']); | ||
$this->submission = Services::get('submission')->get($userVars['submissionId']); | ||
|
||
if (!$this->article) { | ||
if (!$this->submission) { | ||
$request->getDispatcher()->handle404(); | ||
} | ||
|
||
$this->publication = !empty($userVars['publicationId']) | ||
? Services::get('publication')->get($userVars['publicationId']) | ||
: $this->article->getCurrentPublication(); | ||
: $this->submission->getCurrentPublication(); | ||
|
||
if ($this->article) { | ||
if ($this->submission && !CitationStyleLanguagePlugin::isApplicationOmp()) { | ||
$issueDao = DAORegistry::getDAO('IssueDAO'); | ||
// Support OJS 3.1.x and 3.2 | ||
$issueId = method_exists($this->article, 'getCurrentPublication') ? $this->article->getCurrentPublication()->getData('issueId') : $this->article->getIssueId(); | ||
$issueId = method_exists($this->submission, 'getCurrentPublication') ? $this->submission->getCurrentPublication()->getData('issueId') : $this->submission->getIssueId(); | ||
$this->issue = $issueDao->getById($issueId, $context->getId()); | ||
} | ||
|
||
// Disallow access to unpublished submissions, unless the user is a | ||
// journal manager or an assigned subeditor or assistant. This ensures the | ||
// article preview will work for those who can see it. | ||
if (!$this->issue || !$this->issue->getPublished() || $this->article->getStatus() != STATUS_PUBLISHED) { | ||
if ($this->submission->getData('status') !== STATUS_PUBLISHED | ||
|| (!CitationStyleLanguagePlugin::isApplicationOmp() && !$this->issue ) | ||
|| (!CitationStyleLanguagePlugin::isApplicationOmp() && !$this->issue->getPublished())) { | ||
$userCanAccess = false; | ||
|
||
if ($user && $user->hasRole([ROLE_ID_SUB_EDITOR, ROLE_ID_ASSISTANT], $context->getId())) { | ||
$isAssigned = false; | ||
$userGroupDao = DAORegistry::getDAO('UserGroupDAO'); | ||
$stageAssignmentDao = DAORegistry::getDAO('StageAssignmentDAO'); | ||
$assignments = $stageAssignmentDao->getBySubmissionAndStageId($this->article->getId()); | ||
$assignments = $stageAssignmentDao->getBySubmissionAndStageId($this->submission->getId()); | ||
foreach ($assignments as $assignment) { | ||
if ($assignment->getUser()->getId() !== $user->getId()) { | ||
continue; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
{* How to cite *} | ||
{if $citation} | ||
<div class="item citation"> | ||
<section class="sub_item citation_display"> | ||
<h2 class="label"> | ||
{translate key="submission.howToCite"} | ||
</h2> | ||
<div class="value"> | ||
<div id="citationOutput" role="region" aria-live="polite"> | ||
{$citation} | ||
</div> | ||
<div class="citation_formats"> | ||
<button class="cmp_button citation_formats_button" aria-controls="cslCitationFormats" aria-expanded="false" data-csl-dropdown="true"> | ||
{translate key="submission.howToCite.citationFormats"} | ||
</button> | ||
<div id="cslCitationFormats" class="citation_formats_list" aria-hidden="true"> | ||
<ul class="citation_formats_styles"> | ||
{foreach from=$citationStyles item="citationStyle"} | ||
<li> | ||
<a | ||
aria-controls="citationOutput" | ||
href="{url page="citationstylelanguage" op="get" path=$citationStyle.id params=$citationArgs}" | ||
data-load-citation | ||
data-json-href="{url page="citationstylelanguage" op="get" path=$citationStyle.id params=$citationArgsJson}" | ||
> | ||
{$citationStyle.title|escape} | ||
</a> | ||
</li> | ||
{/foreach} | ||
</ul> | ||
{if count($citationDownloads)} | ||
<div class="label"> | ||
{translate key="submission.howToCite.downloadCitation"} | ||
</div> | ||
<ul class="citation_formats_styles"> | ||
{foreach from=$citationDownloads item="citationDownload"} | ||
<li> | ||
<a href="{url page="citationstylelanguage" op="download" path=$citationDownload.id params=$citationArgs}"> | ||
<span class="fa fa-download"></span> | ||
{$citationDownload.title|escape} | ||
</a> | ||
</li> | ||
{/foreach} | ||
</ul> | ||
{/if} | ||
</div> | ||
</div> | ||
</div> | ||
</section> | ||
</div> | ||
{/if} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're going to end up with this template code (and the CSS code) duplicated across OJS/OMP. But each application will have it stored in a different place. Let's try to bring these into alignment if possible. I like that your template/css code is stored in the plugin. Let's see if we can duplicate this in OJS as well. One way to do that is to include this template from the
That way both will be loading from the plugin. We can then remove the CSS styles from OJS's default theme and rely on the ones in this plugin. We should do the same with the JS code that makes it go (this is in the default theme right now). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fine with me! Do you change the article_details.tpl? I will do it the same way in the template for the OMP chapter landing pages, what I'm also working on. Are you sure, that there is JS code for this plugin in the theme? I thought it's all in the plugin. (see here) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right! I thought that this used the dropdown JavaScript code from the default theme but it looks like it doesn't. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to be a little more careful about how we're interacting with non-default themes. This would override the theme's existing font selection. I can think of a couple ways around this:
I'm happy to talk through these options with you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer option one, because then the syle for the plugin is in the plugin. I've made some changes in the css file, so there shouldn't be problems with non-default themes any more. Only font size for the names of the citation styles in the dropdown and neccessary instructions for the arrow by FontAwesome are set by the plugin, now. Color instructions are also only set for some borders of dropdown.