Skip to content
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

Plugin is compatible only with Bootstrap3 #3

Open
ctgraham opened this issue Oct 14, 2021 · 2 comments
Open

Plugin is compatible only with Bootstrap3 #3

ctgraham opened this issue Oct 14, 2021 · 2 comments
Assignees

Comments

@ctgraham
Copy link
Member

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:

$success = parent::register($category, $path, $mainContextId);
if (!$success) return false;
if ($success && $this->getEnabled()) {

Specifically, it is critical not to hook TemplateResource::getFilename if our template overrides are not going to be compatible with the selected theme.

HookRegistry::register('TemplateResource::getFilename', array($this, '_overridePluginTemplates'), HOOK_SEQUENCE_CORE);

The documentation should be clarified here:

* The plugin will function with other themes, but the structure of the blocks conforms to bootstrap3 expectations

@felixhelix
Copy link

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
Copy link
Member Author

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.

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

No branches or pull requests

3 participants