search

ulsdevteam / inlineHtmlGalley

Extend the PKP OJS htmlArticleGalley plugin to support inline display of the HTML Galley
GNU General Public License v2.0
5 stars 5 forks source link

Plugin is compatible only with Bootstrap3 #3

Open ctgraham opened 3 years ago

ctgraham commented 3 years ago

Because we override frontend/components templates, this plugin is really only compatible with Bootstrap3 subthemes.

The frontend/components templates are not core to this plugin and might be considered spurrious. These should be migrated upstream to bootstrap3 as PRs or subthemes if possible.

Until such is done, we need to check in the register method to ensure Bootstrap3 is the enabled theme, and skip any practical effect if not: https://github.com/ulsdevteam/inlineHtmlGalley/blob/d10cc31716c491fb13a4ec960f0e704ba2c7c4ed/InlineHtmlGalleyPlugin.inc.php#L24-L26

Specifically, it is critical not to hook TemplateResource::getFilename if our template overrides are not going to be compatible with the selected theme. https://github.com/ulsdevteam/inlineHtmlGalley/blob/d10cc31716c491fb13a4ec960f0e704ba2c7c4ed/InlineHtmlGalleyPlugin.inc.php#L81

The documentation should be clarified here: https://github.com/ulsdevteam/inlineHtmlGalley/blob/d10cc31716c491fb13a4ec960f0e704ba2c7c4ed/README.md?plain=1#L10

felixhelix commented 1 year ago

Hi @ctgraham

We had two broken journals running the default theme because this plugin was automatically enabled and does not work with this theme. It took me some time to figure out what had caused this problems. It would be extremely helpful to at least change the plugin descriptions and the README.md to make clear that this only works with botstrap themes. Also the plugin (and folder) name suggests that it is applicable for all themes. And the plugin should definitely not be automatically enabled. I could offer to make a pull request for the locale files. I am not sure if and how the plugin name could or should be changed?

Yours, Felix

ctgraham commented 1 year ago

Hi, @felixhelix , the filesystem name for the plugin and folder are intended for identification only, not for information, so I don't anticipate any changes there.

Please feel free to open a PR against this issue to clarify the locale files and README.

I've tagged @wopsononock here regarding the coding changes described above.