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

Added option to active multiple templates #371

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

choules
Copy link

@choules choules commented Feb 14, 2019

This change enables the user to select more than one template in the config. A button in the pagetools gets created for each enabled template.

Copy link
Collaborator

@Klap-in Klap-in left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have a look at the comments.

If it is workable for you that the order cannot be set in the config, current approach is sufficient. If this order is relevant, I think another config settings is the only way.

global $INPUT;
$requestedTemplate = $INPUT->str('template', $templates[0]); // exp

$this->tpl = $requestedTemplate;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include all this logic in the loadExportConfig() function. The getExportConfig() should return the value which is used during the generation of the pdf.

btw, template is the wrong url-parameter, for the template this is actually tpl. But if you update loadExportConfig() this line is not needed anymore.

. '</li>'
) +
array_slice($event->data['items'], -1, 1, true);
foreach($this->getExportConfig('template') AS $template){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use here $this->getConf('template'), not getExportConfig(). Please note that $this->getConf('showexportbutton') is also only configurable from configuration manager. It is not useful to change the GUI by looking to the url-parameter value of tpl.

@@ -6,7 +6,7 @@
$meta['toc'] = array('onoff');
$meta['toclevels'] = array('string', '_pattern' => '/^(|[1-5]-[1-5])$/');
$meta['maxbookmarks'] = array('numeric');
$meta['template'] = array('dirchoice', '_dir' => DOKU_PLUGIN . 'dw2pdf/tpl/');
$meta['template'] = array('multicheckbox', '_other' => 'exists', '_choices' => array_map(function($path) { return basename($path); }, glob(DOKU_PLUGIN . 'dw2pdf/tpl/*', GLOB_ONLYDIR)));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Admin can not set the order of the selected templates. So the first template is, for people using multiple templates, relative random the first of these selected templates.

I like the automatic addition of all available templates.

@@ -984,6 +993,8 @@ public function addsvgbutton(Doku_Event $event) {
return;
}

array_splice($event->data['items'], -1, 0, [new \dokuwiki\plugin\dw2pdf\MenuItem()]);
foreach(explode(',', $this->getExportConfig('template')) AS $template){
array_splice($event->data['items'], -1, 0, [new \dokuwiki\plugin\dw2pdf\MenuItem($template)]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer that the MenuItem($template) is only set to a specific template, if it is needed. So if more than one template is set in the config 'template'.


$suffix = '';

if(strlen($this->template) > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strlen() is not needed to check whether $this->template is empty.

@@ -6,7 +6,7 @@
$conf['toc'] = 0;
$conf['toclevels'] = '';
$conf['maxbookmarks'] = 5;
$conf['template'] = 'default';
$conf['template'] = array('default');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a string.
Since I did not save yet my config manager, this throws a warning, because the conf returns the default array instead of a string.

PHP Warning: explode() expects parameter 2 to be string, array given in /path/dokuwiki/dokuwiki/lib/plugins/dw2pdf/action.php on line 37

@Klap-in Klap-in marked this pull request as draft August 21, 2020 18:39
@Klap-in Klap-in marked this pull request as ready for review August 21, 2020 18:39
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

Successfully merging this pull request may close these issues.

2 participants