Remove the extra base path variable from the default theme

[immanuelfodor]

The base path variable in the default theme is extraneous and it breaks updated plugin asset paths (instead of /plugins these become //plugins which is another thing).

• https://github.com/shaarli/Shaarli/blob/master/tpl/default/includes.html#L15
• https://github.com/shaarli/Shaarli/blob/master/tpl/default/page.footer.html#L28

Referencing the discussion under ilesinge/shaarli-related#11

[ArthurHoaro]

I'm not sure about this one. Don't you think it would be easier to keep the base path variable in the templates? It would avoid having to add the base path in every template, and preserve backward compatibility with previous version: no change required to include CSS and JS files.

ping @kalvn and @ilesinge for other opinions.

Just to sum it up, whether we keep:

# in plugins
$data['js_files'][] = PluginManager::$PLUGINS_PATH . '/demo_plugin/demo_plugin.js';
# in templates
<script src="{$base_path}/{$value}#"></script>

Or we do not assume anything in templates:

# in plugins
$data['js_files'][] = ($data['BASE_PATH'] ?? '') . '/' . PluginManager::$PLUGINS_PATH . '/demo_plugin/demo_plugin.js';
# in templates
<script src="{$value}#"></script>

Note that $data['BASE_PATH'] will still be required for other usage, e.g. in QRCode plugin to include the image.

[immanuelfodor]

I added this issue as I exclusively use the Material theme and it used the asset includes this way. It could give more flexibility to plugins, however, it required me changing all my plugins and also submit pull requests to other authors already, so I'm in favour of just passing the value instead of reverting all the changes 😃 Maybe it was wrong for all the years and missed the base path, so it should be adjusted and plugin basepath additions reverted anyways. The plugin path const is for the filesystem, so transforming it to url in the plugin seems weird though.

I think anything is decided here, should be added to the theme / plugin development docs as usage policy for the future.

[kalvn]

I think it's something I forgot in the Material theme, I should add the base path for plugin JS and CSS files. So I would be rather in favor of @ArthurHoaro first option. Plugin developer shouldn't care too much about base path. They have PluginManager::$PLUGINS_PATH available and they should trust it. I'll update Shaarli Material (and my plugins) in that sense.

Now for edge case, what do you think of providing new elements in $data for including external CSS and JS files in a plugin? I'm thinking about files stored on another domain for example.

Something like:

$data['js_files'][] = PluginManager::$PLUGINS_PATH . '/demo_plugin/demo_plugin.js';
$data['js_files_external'][] = 'https://cdn.whatever.com/super_plugin/super_plugin.js';

<!-- For each internal script -->
<script src="{$base_path}/{$value}#"></script>
<!-- For each external script -->
<script src="{$value}#"></script>

Writing it, it sounds like increasing complexity for no real benefit. I'm not sure.

[ilesinge]

"and preserve backward compatibility with previous version: no change required to include CSS and JS files." => I'm for the 1st option too for simplicity and compatibility sake.
"I think anything is decided here, should be added to the theme / plugin development docs as usage policy for the future." => Agreed.
"Now for edge case, what do you think of providing new elements in $data for including external CSS and JS files in a plugin?" => Do you have a real use case in mind ? I'd say YAGNI, and as a self-hosted app admin, I prefer that there are no potential leaks to 3rd party domains, so I would not encourage that in plugins.

[ArthurHoaro]

Thank you everyone for your feedbacks. Comments here are mostly in favor of keeping the base path in the templates, so we will go with it. I'm sorry to have lead you @immanuelfodor into unnecessary changes.

I agree that it should be clearly explained in the documentation though. I'll add it and fix core plugin in the next days.

[immanuelfodor]

No problem at all, finally it got sorted out how to handle it the right way, I'll revert back when you and @kalvn is ready with the changes, please ping me here when done.

[kalvn]

@immanuelfodor I updated Shaarli Material and my plugins accordingly :)

[immanuelfodor]

Thank you very much! I'll wait for Arthur's ping as well, I wouldn't want to break my docker image 😃

[immanuelfodor]

Updated all my plugins, they now work according to PR #1558