idea draft: template driven page list icons

[Martijn Geerts]

I do like your module. Thats for sure.

Here's my reply just for the discussion The color issue is no issue, if you use it as pseudo element you could use Soma's beautiful color picker. (love that one) Think there are a lot of opensource icon fonts, although the size would be bigger then all the icons you would use together. But there would be only 1 request instead, think it's quicker.

About the older browsers I should agre when it's in the open, but in the admin think it's a good practice to learn editors to use a modern browser. (Maybe even a good idea to block IE7 & lower & serve links to alternatives on PW back-end, as I think that could increase functionality with cleaner code )

[apeisa]

Martijn: IE7 and lower are already blocked. IE8 is currently lowest allowed and hopefully Win XP just fades away soon enough so that we can forget about it. Or MS makes the IE9 to work in XP too (which I very much doubt, it would have happened already).

[Soma]

<= IE7 is blocked from the admin anyway already by the admin theme javascript.

I don't think ti would be a problem to use icon fonts, but hard to implement a module based on what chars and how the icon font is made. It would be a horror to make your own and further.

If you look at the page tree and what you can customize, and quickly create your own icon for a special purpose, dorp that in the folder and go... is far more flexible than hard molted icon font.

I agree it would be a nice thing for buttons and other UI elements in PW admin, and use icon font for a custom theme.

[Martijn Geerts]

Mapping those fonts is indeed tricky and surely not that obvious as doing it with images... Didn't know that IE7 already bocked, thats a good thing. As apeisa mentioned XP sticks on IE8.

[Soma]

Thanks for your input, Soma.

For now, I decided to simply fork your theme and get rid of the icons - I did two commits on a fork here, one that removes the icons, and another that renders the lock/unlock icons in black. (note that there's also a "key" icon in PW PageList module, which does not appear to be in use??)

Open template, remove guest role... configure. Tadaa, key icon show for pages using that template. Must have dreamed or confused with my current icons

Ok, just now reading again this post I have previously missed the fork link. Was too tired reading stuff. Will check out later.

I don't want my module to contain workarounds for any specific themes - if other themes add icons or something else, going down that path leads to increasing maintenance effort.

Yeah, any dependency should be left out from both sides if possible. I think my solution of preventing the icons output by adding a check in the theme to see if the you page list icon module is installed or not, would be a good and easy one. Simply add a CSS file or not.

I agree, separate icons for states that are already reflected by typography or opacity, not necessary. It would be nice to have icon choices for the "system" pages though - e.g. admin, trash, etc... although I suppose you could already turn on display of "system" templates and simply add the icons to them? So perhaps this is not needed at all. Might be nice if the installer configured those for you on install though - I looked into that, but I'm not sure how to configure a module at install... (is that possible?)

Well "system" templates is actually one "admin" template for all admin pages. So no. Maybe a different solution would be possible there, but I see none.

Yes you can, use ___install().

[Soma]

About InputFieldMarkup for the icons - won't this cause the icons to go in a separate group from the dropdown control? That's what I was trying to avoid - that was the only reason they were mangled together like that.

To come back to this.

What I do is this:

$markup = $this->modules->get('InputfieldMarkup');
$markup->label = $this->_('Page Icon');
$markup->value = $icon->render();

$markup->value .= '<div id="template-badge-icons">';
foreach ($icons as $_icon => $name) {
 $markup->value .= '<img title="'.htmlspecialchars($name).'" src="'.$icons_url.$_icon.'" data-icon="'.$_icon.'" />';
}
$markup->value .= '</div>';
if (file_exists($icons_license)) {
 $markup->value .= file_get_contents($icons_license);
}

$t->add($label);
$t->add($color);
$t->add($markup);

And you get your desired output A fieldset with the select and the icons inside it.

[mindplay.dk]

And you get your desired output A fieldset with the select and the icons inside it.

I'm not sure I fully understand how this works yet, but now your icon selector is no longer part of the form - it's just being rendered and the markup gets inserted somewhere. Does that work? Will it still get processed properly, initialized with it's value and updated on POST and so forth?

[mindplay.dk]

Did some tweaks to the master branch today, and tagged version 1.0.3 - you may consider this the first "stable" release, if you please

I also started a 1.0.4-dev branch, in which I've refactored the icon-selector into an Inputfield-type, which you can actually install and use without installing the TemplateDecorator, if you like. It works, but needs a better UI, probably a popup for the icon-selector, since it takes up a lot of space. (thoughts?)

As mentioned, I also plan on refactoring the preset color-picker into an Inputfield-type - and when that's done, I will probably add support for the existing RGB color-picker, which I won't personally use for this, but it would be easy to support, so why not

I also plan on getting some sleep tonight

[mindplay.dk]

Regarding installation, I tried the following:

public function ___install()
{
 $this->default_icon = 'page.png';
 $this->icon_adminRootPageID = 'config.png';
 $this->icon_trashPageID = 'trash.png';
 $this->icon_http404PageID = 'bug.png';
 $this->show_template = true;
}

Didn't work. Any ideas?

EDIT: figured it out

   public function ___install()
   {
    wire('modules')->saveModuleConfigData($this->className(), array(
	    'default_icon' => 'page.png',
	    'icon_adminRootPageID' => 'config.png',
	    'icon_trashPageID' => 'trash.png',
	    'icon_http404PageID' => 'bug.png',
	    'show_template' => true,
    ));
   }

[Soma]

I'm not sure I fully understand how this works yet, but now your icon selector is no longer part of the form - it's just being rendered and the markup gets inserted somewhere. Does that work? Will it still get processed properly, initialized with it's value and updated on POST and so forth?

Why shouldn't it work? I just use the InputfieldMarkup to create the "wrapper" fieldset and then I add "rendered" markup to it ie. the select inputfield. I removed the label from the select inputfield because the label is now defined by the IntpufieldMarkup. It's how we do this in PW and nothing special here.

So first prepare the InputfieldSelect and add the output to it. Later add the icons container markup to it and add the markup to the form.

   $icon = $this->modules->get('InputfieldSelect');
   $icon->addOption('');
   $icon->attr('id+name', 'badge_icon');
   $icon->attr('value', $template->badge_icon);
   $icon->addOptions($icons);

   $markup = $this->modules->get('InputfieldMarkup');
   $markup->label = $this->_('Page Icon');
   $markup->value = $icon->render(); // add markup of the select input field
... add more to it

Did some tweaks to the master branch today, and tagged version 1.0.3 - you may consider this the first "stable" release, if you please

I also started a 1.0.4-dev branch, in which I've refactored the icon-selector into an Inputfield-type, which you can actually install and use without installing the TemplateDecorator, if you like. It works, but needs a better UI, probably a popup for the icon-selector, since it takes up a lot of space. (thoughts?)

As mentioned, I also plan on refactoring the preset color-picker into an Inputfield-type - and when that's done, I will probably add support for the existing RGB color-picker, which I won't personally use for this, but it would be easy to support, so why not

I also plan on getting some sleep tonight

I can't follow why you do this but it's up to you. I don't think an extra intpufield is needed inside this module, if, it should be separate inputfield. However I don't really care as I got my own PageListIcon module which I will use, as your module doesn't work on most of my clients hosting.

[Soma]

Regarding installation, I tried the following:

public function ___install()
{
 $this->default_icon = 'page.png';
 $this->icon_adminRootPageID = 'config.png';
 $this->icon_trashPageID = 'trash.png';
 $this->icon_http404PageID = 'bug.png';
 $this->show_template = true;
}

Didn't work. Any ideas?

EDIT: figured it out

public function ___install()
{
 wire('modules')->saveModuleConfigData($this->className(), array(
	 'default_icon' => 'page.png',
	 'icon_adminRootPageID' => 'config.png',
	 'icon_trashPageID' => 'trash.png',
	 'icon_http404PageID' => 'bug.png',
	 'show_template' => true,
 ));
}

That's fine. Glad you figured it out.

Just wanted to mention an alternative we usually use is we create some default config array to use in a module, then merge it with the saved in getModuleConfigInputfields().

static protected $defaults = array(
'myicon' => "hello.jpg"
);

Then in the config method.

static public function getModuleConfigInputfields(array $data) {
   $data = array_merge(self::$defaults, $data);
...

And you could use that default config in the module if nothing else is defined yet.

[mindplay.dk]

Why shouldn't it work? I just use the InputfieldMarkup to create the "wrapper" fieldset and then I add "rendered" markup to it ie. the select inputfield. I removed the label from the select inputfield because the label is now defined by the IntpufieldMarkup. It's how we do this in PW and nothing special here.

It just doesn't feel right somehow - throwing away the model and keeping the markup. It might work in this particular case, but it doesn't seem like a good long-term solution for modules where you might want to keep a form open to extension - once you've rendered the field, it's no longer mutable. Just feels like a work-around to me...

I don't think an extra intpufield is needed inside this module, if, it should be separate inputfield.

The extra Inputfields would be useful for whatever you want to use them for, same as any other Inputfield - for example, you could add icons or preset color-pickers to your own templates, e.g. for front-end. Icons might be useful e.g. for menus on a site, or icons on download-links, etc.

Mostly I'm just doing it as rehearsal - teaching myself more about PW... and it can't do any harm, so why not?

[Soma]

It just doesn't feel right somehow - throwing away the model and keeping the markup. It might work in this particular case, but it doesn't seem like a good long-term solution for modules where you might want to keep a form open to extension - once you've rendered the field, it's no longer mutable. Just feels like a work-around to me...

I understand what you mean, but I don't think you'll ever need it and makes no sense to create another module hook to change the icon select. Even with your method I don't think is any different. As I said I understand, and it certain cases this might be desired, but not here.

However then you'll find ProcessWire maybe "not right" because it uses this technique in many cases. Even the buidlForm of ProcessTemplate has rendered markup in there added to the form.

[ryan]

It's best to let an entire form be rendered together as a group, when possible. Like if you call $form->render(), that should render all Inputfields within the form together. This is important because it also has to process [input] them as a group. So if there are pre-rendered Inputfields in a form, they won't get processed with the form. So it creates more things for you to keep track of if you don't have all your fields rendering and processing together as a group. It's true that it's also easier to hook and extend these things if they aren't already rendered into markup. The main exception is the InputfieldMarkup type. That's designed to hold rendered markup for things that can't necessarily be rendered together with the form, like a table or some other kind of markup unknown to Inputfields. The example Soma is talking about (I think) is the MarkupAdminDataTable in ProcessTemplate that gets rendered into an InputfieldMarkup. Longer term, maybe we could come up with a new "Renderable" interface (requiring a class to just have a render() method that returns markup) so that stuff like this could be rendered together with the form fields.

[mindplay.dk]

This is important because it also has to process [input] them as a group. So if there are pre-rendered Inputfields in a form, they won't get processed with the form.

This is what I was referring to.

But anyhow, I decided to go the more adventurous route and learn how to build an Inputfield-type - I haven't attempted to use my icon-selector with a Field on a Template yet, but my hope is that the icon-selector and preset color-picker would be useful for more than just the TemplateDecorator itself.

To me, this is one of the most important values in ProcessWire: the idea of building small, reusable features - as opposed to large, self-contained concepts... I believe this is the core reason why platforms such as Drupal or WordPress need hundreds of similar modules for the same things, to satisfy everyone - when you build out an entire concept, it's set in stone, and it doesn't fit everyone's needs, so every shop-module (for example) addresses a slightly different set of needs. When we provide the building-block features instead of self-contained concepts, this enables the developer to rapidly apply those features to solution-specific concepts. The module-concept in most other CMS suffer (by design) from what I've been calling "conceptual lock-in" for some years now - PW doesn't suffer from this, and as such will hopefully see smaller numbers of more lightweight and generic modules, from which developers can design the concepts.

(sorry for side-tracking, but I had to put that in writing!)

[Soma]

Adding the rendered markup to the form will not be different from the

This is what I was referring to.

Sorry, just have to get that right. An inputfield prerendered and added to the form will actually work as in my example. Not sure now what's with that it won't get processed as Ryan mention, because it obviously does work. The value get's saved and populated as it is also the module itself taking care of it. The module provided the inputs and only itself know how to handle/save it and not the form.

But I get how this limits future modifications of the Inputfield as it's already rendered. And while having a need for additional markup that can't be done by a Inputfield there's the InptufieldMarkup. Or to make it again modular, there's only the way of having it be new Inputfield again that can be used.

I think I got learned a lot just from this thread again, good times!

[ryan]

Not sure now what's with that it won't get processed as Ryan mention, because it obviously does work.

It only won't get processed if you are asking the form to process the input, i.e. $form->processInput($input->post); When you do that, it goes through all the fields and processes them. From there, you can retrieve any value from it, which is now a validated value. Like this: $value = $form->get('your_field_name')->attr('value'); So if you had a pre-rendered input in an InputfieldMarkup, that would not have been processed with the form, nor could you retrieve it specifically from the form (since it only knows it as markup).

However, I sometimes use Inputfield forms just for markup generation, and I grab the resulting value from $input->post (or $_POST) myself... Especially for something like an integer or checkbox where I don't need any validation beyond just type casting. In this case, it really doesn't matter how or where your inputs were rendered, because you are using the form just for markup generation, not processing input. There is still the benefit of being more hookable and extendable if you let the form render as a group though. But there are sometimes instances where I have some one-off input need that I can only accomplish easily with actual markup, like this one below, so it's InputfieldMarkup all the way there.

[mindplay.dk]

@ryan - I wanted to display the icons on the template admin page as well, since with a large number of templates, it's useful to have an overview of the icons you're currently using. Unfortunately, the ProcessTemplate::___execute() hook is not very useful:

 foreach($this->fuel('templates') as $template) {
  $notes = '';
  if($template->flags & Template::flagSystem) {
   if(!$filterSystem && !$filterField) continue;
   $fieldset->collapsed = Inputfield::collapsedNo;
   $notes .= $this->_x('system', 'list-note') . ' ';
  }
  if($filterField && !$template->fieldgroup->has($filterField)) continue;
  if($template->useRoles) $notes .= ' ' . $this->_x('access', 'list-note');
  if(!$template->filenameExists()) $notes .= ' ' . $this->_x('no-file', 'list-note');
  $table->row(array(
   "{$template->name} " => "edit?id={$template->id}",
   $template->label,
   $template->getNumPages(),
   count($template->fieldgroup) . ($template->fieldgroup->name != $template->name ? " {$template->fieldgroup->name}" : ''),
   $notes
   ));
 }

Perhaps another hook could be added, like ___addTemplateRow() or something:

function ___getTemplateRow($table, $template, $notes) {
  return array(
   "{$template->name} " => "edit?id={$template->id}",
   $template->label,
   $template->getNumPages(),
   count($template->fieldgroup) . ($template->fieldgroup->name != $template->name ? " {$template->fieldgroup->name}" : ''),
   $notes
   );
 }
}

I don't know if that's very versatile or clever - perhaps you can think of a better way to do this?

I would hate to have to resort to something like parsing/searching the HTML snippet...

[ryan]

I like your idea of adding that. I'd also need to add another function for the table header too. Though wondering if there might be more mileage in just making the MarkupAdminDataTable methods hookable, as that would provide the same flexibility anywhere these tables are used (though with a little extra code to make sure you are hooking the right table, like below).

Theoretical code (this would not work at present since MarkupAdminDataTable::row is not yet hookable)

public function ready() {
 if($this->process == 'ProcessTemplate') $this->process->addHookBefore('execute', $this, 'hookTemplateList'); 
}

public function hookTemplateList(HookEvent $event) {
 $this->addHookBefore('MarkupAdminDataTable::row', $this, 'hookTemplateListTable'); 
}

public function hookTemplateListTable(HookEvent $event) {
 $arguments = $event->arguments; 
 $row = $arguments[0];
 $row[] = 'Another column';
 $arguments[0] = $row;
 $event->arguments = $arguments; 
}

[mindplay.dk]

Hmm, but I don't want to hook every MarkupAdminDataTable::row() call - just for that particular instance.

Perhaps the simpler solution is to add a hookable createTable() function that takes the list of templates as argument, and returns the MarkupAdminDataTable instance?

That way, you could choose whatever approach works best for a given situation - either hooking the row() method of just that instance using a before-hook, or iterating through the rows in an after-hook.

[mindplay.dk]

On another note, @ryan - looks like TemplateDecorator still can't work with PHP prior to 5.3, because I'm using ReflectionProperty::setAccessible() in this method:

https://github.com/mindplay-dk/TemplateDecorator/blob/master/TemplateDecorator.module#L154

Would you consider exposing the property, so I can make my module work with PHP 5.2?

[Soma]

On another note, @ryan - looks like TemplateDecorator still can't work with PHP prior to 5.3, because I'm using ReflectionProperty::setAccessible() in this method:

https://github.com/m...tor.module#L154

Would you consider exposing the property, so I can make my module work with PHP 5.2?

I already provided and example to make that code simpler and working with php < 5.3...

https://github.com/mindplay-dk/TemplateDecorator/issues/3#issuecomment-8997263

[mindplay.dk]

Right, thanks @somatonic - I had forgotten... Just integrated that change, seems to work

[ryan]

Hmm, but I don't want to hook every MarkupAdminDataTable::row() call - just for that particular instance.

In the example above, it would actually only hook into rows for ProcessTemplate when it is doing the template list (execute) function.

Perhaps the simpler solution is to add a hookable createTable() function that takes the list of templates as argument, and returns the MarkupAdminDataTable instance?

This could be a good way to go. There are several instances of tables in the admin, so was just trying to think of an overall solution that would apply to all tables rather than one just for ProcessTemplate. That way the next time someone asks how they can hook into a table, I'll be able to give a better answer than I could this time.

On another note, @ryan - looks like TemplateDecorator still can't work with PHP prior to 5.3, because I'm using ReflectionProperty::setAccessible() in this method:

Since when do you care about PHP prior to 5.3? Between you and SiNNuT, I've been convinced we need to exit 5.2 sooner rather than later. GitHub isn't loading for me right now so can't see what the link is pointing to, but I'll be happy to make any adjustments necessary.

[mindplay.dk]

Since when do you care about PHP prior to 5.3?

Ha ha, I don't personally - haven't used 5.2 since 2010 ... But someone quickly posted a "bug report", and apparently I love working for free - isn't that the spirit of open source?