Add default styling for HTML galleys

[Vitaliy-1]

The new public method in the ThemePlugin class will allow the user to upload CSS/LESS as a default styling for the HTML galley.

Jan 7, 2019: This has been implemented for OJS. It still needs to be ported for use in OMP, and the hook names updated appropriately. It also needs to be documented in the PKP Theming Guide.

[Vitaliy-1]

I can be assigned to this issue.
@NateWr I have a question on this: is there a reason why the {load_script context="frontend"} is absent in the HTML galley's display.tpl? https://github.com/pkp/ojs/blob/master/plugins/generic/htmlArticleGalley/display.tpl
Downloading Theme's javascript on that page is unwanted?

[NateWr]

It was probably just because it's a template that doesn't show the "normal" website layout. But you could probably add it back in, or consider using a unique context so that it's easy to add styles of JS specifically to this page:

{load_script context="galley"}

[Vitaliy-1]

@NateWr
I'm finishing modifications for this enhancement. They include adding a method inside ThemePlugin class and a hook inside HTML Galley plugin for making modifications inside displayed article's HTML. Is it possible to put it to one pull request, and how I can do it? Or should I ship them separately?

[NateWr]

@Vitaliy-1 The HTML Galley plugin is part of the OJS repository, rather than a separate submodule. So you can make changes to that and put them all in the same commit/PR to OJS. 👍

[Vitaliy-1]

ThemePlugin class is inside pkp-lib submodule and I didn't make submodule update before. What is the algorithm here?

[NateWr]

Ok, this is always a bit of a hurdle the first time you do it. So let's suppose the following (adjust according to your branch names):

• Your OJS changes are in your branch, i3691_galley_styles.
• Your PKP changes are in your branch, i3691_galley_styles.

Do the following:

1. Make sure both repositories are up-to-date with the latest master.
2. Commit all changes to both.
3. In your OJS dir, run the following:

git add lib/pkp
git commit -m "Submodule update ##Vitaliy-1/i3691_galley_styles##"
git push

The syntax of the commit message is:

Submodule update ##<username>/<branchname>##

That tells the testing suite to check out your repository and branch when running the tests.

Let me know if you have any difficulties. 🤞

[Vitaliy-1]

Hmm, don't know git well.
I have "local" and "origin" (my fork) ojs and pkp-lib repositories that are up to date and have i3691_galley_styles branch with needed changes. Also, they have original pkp repos as upstream. Or should I have only ojs repostitory with updated submodules (git submodule update --recursive) and make changes in ThemePlugin class there?

Right now git add lib/pkp and making a commit returns On branch i3691_galley_styles nothing to commit, working tree clean, which is expected and git push returns fatal: The current branch i3691_galley_styles has no upstream branch, and it is true because upstream (in this case original ojs repository) doesn't contain this branch. Should I push here specifically git push origin i3691_galley_styles or do something else?

[NateWr]

You'll want to have i3691_galley_styles checked out in your lib/pkp repo. So I would do something like this:

cd <ojs-dir>
git checkout i3691_galley_styles
cd lib/pkp
git checkout i3691_galley_styles
cd ../..
git add lib/pkp
git commit -m "Submodule update ##Vitaliy-1/i3691_galley_styles##"
git push

If in this series, after doing git add lib/pkp, you get the message that there is nothing to commit, it means one of two things:

1. You have made no commits in your lib/pkp branch.
2. You have already added the lib/pkp changes to a previous commit in your OJS branch.

The second one can happen if you use git add . to make your commits in OJS. This will pick up the submodules and commit those two. But we prefer to keep them separate, and add them in their own commit at the very end.

If that doesn't solve it, let me know and we can do a google hangout.

[NateWr]

I left a couple comments on the PRs. In the future, when you're ready for another look, leave a comment here so I know to check the PRs again.

Also, we like to link to the PRs like this so it's easy for the reviewer to find.

PRs:
#4225
pkp/ojs#2160

[Vitaliy-1]

@NateWr
I've revised the approach today. It seems that for keeping fewer code modifications, it is better to transfer the method that calls HTML's hook in PKPTemplateManager or add a getter there for retrieving URL's for styles to be called from ThemePlugin. I've picked up the former. Hope that new commits were done in the right way :)

[NateWr]

Looks great, @Vitaliy-1! I'm happy to merge. I left one comment but otherwise just needs a rebase and it's ready to go. 👍

[Vitaliy-1]

@NateWr Thanks for the review and help. I've updated this one and pkp/lib sudmodule in ojs rep.
Anything else I should do here?

[NateWr]

It looks like you've still got a merge conflict. This can happen if either ojs or pkp-lib need a rebase. My usual approach is:

1. rebase pkp-lib and force push
2. pull the submodule commit off of ojs (git reset HEAD~1)
3. rebase ojs
4. git add lib/pkp && git commit -m "..." and force push

Let me know if you want cmd-by-cmd on any of that.

[NateWr]

@Vitaliy-1 a reminder that I'm still hoping you'll take a quick look at the code here to make sure it looks alright.

[Vitaliy-1]

Yep, the code looks perfect and it works as expected. My only concern for the hook was that it can be used later by plugins to add data to HTML from OJS.

[NateWr]

👍 Thanks! I thought about that, but I think for themes we'll want to wrap this into the Theme API methods for addStyle, and maybe add an addHeader method, so that theme developers don't have to wrestle with hooks and modifying the HTML content directly.

[schlattk]

Hello, Is this feature now available? I am running version 3.1.1.4 - thanks!

[NateWr]

@schlattk I believe this is only available in 3.1.2+.

[schlattk]

ok thanks @NateWr

[denismaier]

Looks like this should be available by now. Correct? Any hints how this can be used? Is that documented somewhere?

[NateWr]

@denismaier good point, this should be documented as part of the Theme API.

Use it by adding a style to the context htmlGalley:

$this->addStyle('htmlGalley', 'path/to/galley.css', ['contexts' => 'htmlGalley']);

[denismaier]

@NateWr thanks, I'll have a look. Where should you store the stylesheet?

[NateWr]

With your custom theme plugin.

[denismaier]

Ok. And how do you tefetence the stylesheet in the html?

[NateWr]

You don't. It will be automatically injected into the <head> of the HTML. The goal of this feature is to separate out the style (specific to a theme) from the galley (which will persist over time and through journal branding changes).

[denismaier]

I see. For testing and development purposes I nevertheless have a link to a stylesheet in the html. Do I have to remove that before uploading? Or will that just be ignored or overriden?

[NateWr]

This feature injects a <link> tag into the <head> of the HTML. If won't remove or overwrite any existing tags. So if you have a <link> tag in your HTML, the browser will still try to fetch that CSS file. I recommend you remove it from your existing HTML.

[denismaier]

I've now finally managed to try this---without much success though.

Here's what I've done:

1. Set up a new OJS instance (version 3.1.1.4; that's the version we use in production. Should be ok, right?)
2. Set up a new journal, publish an article in new issue
3. Add a file galley.css in the default theme folder (in the styles folder).
4. In DefaultThemePlugin.inc.php I've added the code snippet @NateWr has provided above:

   $this->addStyle('htmlGalley', 'styles/galley.css', ['contexts' => 'htmlGalley']);

Now, this should use my css for the galley, right? But this doesn't happen. Moreover, upon investigating the source code I found no traces of an injected <link> to the css stylesheet in the <head> of the HTML.

What am I missing?

[Vitaliy-1]

This works from OJS 3.1.2+, I've also backported it to the OJS 3.1.1 stable branch but as far as I remember it was after the release of OJS 3.1.1.4.

[denismaier]

That was it. Thanks.