Item14092: attach.pattern.tmpl
needs a hook for plugins to add properties.
Priority: Enhancement
Current State: Closed
Released In: 2.1.3
Target Release: patch
--
PhilippeKehl - 11 Jun 2016
Plugins that use
beforeUploadHandler()
or afterUploadHandler()
, such as my
PhotoGalleryPlugin, may want to introduce additional properties (request parameters) to manipulate uploads.
The
PatternSkin doesn't provide a hook to add
<form>
elements but one instead must override the whole
properties
definition from the
attach.pattern.tmpl
template. This will be a problem when the
PatternSkin's definition of the checkboxes changes and the plugin that overrides the section doesn't update.
I think the fix is as easy as adding a
%TMPL:P{"propertieshook"}%
to the
properties
definition that plugins can use to inject their own stuff. Using
%TMPL::PREV%
this can even work for multiple plugins. This can be documented in the
attach.pattern.tmpl
file directly.
So
attach.pattern.tmpl
adds
%TMPL:P{"propertieshook"}%
:
%TMPL:DEF{"properties"}%<div class="foswikiFormStep">
---+++ %MAKETEXT{"Properties"}%
<input type="checkbox" class="foswikiCheckbox" id="createlink" name="createlink" %ATTACHLINKBOX% /><label for="createlink">%MAKETEXT{"Create a link to the attached file"}%</label> <span class="foswikiGrayText">%MAKETEXT{"Images will be displayed, for other attachments a link will be created."}%</span>
<input type="checkbox" class="foswikiCheckbox" id="hidefile" name="hidefile" %HIDEFILE% /><label for="hidefile">%MAKETEXT{"Do not show attachment in table"}%</label> <span class="foswikiGrayText">%MAKETEXT{"Attachments will not be shown in topic view page."}%</span>
%TMPL:P{"propertieshook"}%</div>%TMPL:P{"changepropertiesaction"}%%TMPL:END%
%{
The "propertieshook" allows skins to add properties, e.g. by plugins that
use the beforeUploadHandler() or afterUploadHandler() functions to handle
additional properties. Use the %TMPL:PREV% to make those _add_ to the
"propertieshook" placeholder (instead of replacing it) so that multiple
skins can add their properties. E.g.:
%TMPL:DEF{"propertieshook"}%%TMPL:PREV%
<input type="checkbox" class="foswikiCheckbox" id="someprop"/><label for="someprop">%MAKETEXT{"Some property description."}%</label>
%TMPL:END%
}%
And plugins or other skins (e.g.
attach.photogallery.tmpl
) can do something like this:
%TMPL:DEF{"propertieshook"}%%TMPL:PREV%
<input type="checkbox" checked class="foswikiCheckbox" id="exifrotateimage" name="exifrotateimage"/> <label for="exifrotateimage">%MAKETEXT{"Rotate JPEG images based on EXIF tags (using <code>exiftran</code>)."}%</label>
<input type="checkbox" checked class="foswikiCheckbox" id="setexifdate" name="setexifdate"/> <label for="setexifdate">%MAKETEXT{"Set file upload date to EXIF exposure date for JPEG images."}%</label>
%TMPL:END%
Pull request filed:
https://github.com/foswiki/distro/pull/13
--
PhilippeKehl - 11 Jun 2016
MichaelDaum ... Could you review this? Does it rise to the level of a
FeatureRequests or is it simple enough to add on a task.
--
GeorgeClark - 12 Aug 2016
Makes sense. No beauty, but if it helps...
I would have less fear to redefine all of
properties
in a skin overlay.
An alternative approach being used in other parts of the skin is to split up
properties
like this:
%TMPL:DEF{"properties"}%%TMPL:P{"properties::start"}%%TMPL:P{"properties::createlink"}%%TMPL:P{"properties::hidefile"}%%TMPL:P{"properties::end"}%%TMPL:END%
%TMPL:DEF{"properties::start"}%
<div class="foswikiFormStep">
---+++ %MAKETEXT{"Properties"}%
%TMPL:END%
%TMPL:DEF{"properties::createlink"}%
<input type="checkbox" class="foswikiCheckbox" id="createlink" name="createlink" %ATTACHLINKBOX% /><label for="createlink">%MAKETEXT{"Create a link to the attached file"}%</label> <span class="foswikiGrayText">%MAKETEXT{"Images will be displayed, for other attachments a link will be created."}%</span>
%TMPL:END%
%TMPL:DEF{"properties::hidefile"}%
<input type="checkbox" class="foswikiCheckbox" id="hidefile" name="hidefile" %HIDEFILE% /><label for="hidefile">%MAKETEXT{"Do not show attachment in table"}%</label> <span class="foswikiGrayText">%MAKETEXT{"Attachments will not be shown in topic view page."}%</span>
%TMPL:END%
%TMPL:DEF{"properties::end"}%</div>%TMPL:P{"changepropertiesaction"}%%TMPL:END%
A skin overlay would then redefine
properties
and add a
foobar
property using this:
%TMPL:DEF{"properties"}%%TMPL:P{"properties::start"}%%TMPL:P{"properties::createlink"}%%TMPL:P{"properties::hidefile"}%%TMPL:P{"properties::foobar"}%%TMPL:P{"properties::end"}%%TMPL:END%
Just my 2cent.
--
MichaelDaum - 15 Aug 2016
George and Michael, many thanks for your help and comments!
Michael: Your proposed approach is more elegant than the current approach I take (reproducing the whole markup in my skin) but it still requires my skin to "know" what properties::* it should reproduce.
And when there are two skins (plugins) that both want to add properties, it wouldn't work (would it?). Only the "last" (highest priority) skin's props would get added.
Is it the
%TMPL:PREV%
that you dislike?
To me
SkinTemplates and
ReleaseNotes01x01 suggests this use. The only place I can find where it's used today is in
templates/compare.tmpl
to add CSS styles (not sure why it's done that way, though).
Given that I want to support multiple skin overlays adding properties, and not reproducing the original stuff, the only alternative approach I can think of is using some JS magic to move the added markup to the right place run-time.
I could live with redefining all of
properties
in my skin, no problem. But I thought providing some mechanism to
add checkboxes would be beneficial.
What do you think?
--
PhilippeKehl - 15 Aug 2016
%TMPL:PREV
is an excellent extension to the TMPL language. However it only is applicable extending a previous definition in a linear fashion. In the above case we'd like to extend the inner part.
Let's try another structure to facilitate
%TMPL:PREV
:
%TMPL:DEF{"properties"}%<div class='foswikiFormStep'>%TMPL:P{"properties::content"}%</div>%TMPL:P{"changepropertiesaction"}%%TMPL:END%
%TMPL:DEF{"properties::content"}%
%TMPL:P{"properties::title"}%
%TMPL:P{"properties::createlink"}%
%TMPL:P{"properties::hidefile"}%
%TMPL:END%
%TMPL:DEF{"properties::title"}%---+++ %MAKETEXT{"Properties"}%%TMPL:END%
%TMPL:DEF{"properties::createlink"}%...%TMPL:END%
%TMPL:DEF{"properties::hidelink"}%...%TMPL:END%
Your skin can now redefine
properties::content
by making use of
%TMPL::PREV
in a safe way:
%TMPL:DEF{"properties::content"}%%TMPL:PREV%%TMPL:P{"properties::foobar"}%%TMPL:END%
The idea here is to structure the elements in a semantic way so that they make sense on their own. A
propertieshook
definition will be empty in almost all cases except yours. Only then will it make sense.
--
MichaelDaum - 16 Aug 2016
PhilippeKehl, if you could restructure the pull request I'll get your changes merged into master. I'm setting this task as planned for 2.2. Thanks.
--
GeorgeClark - 16 Aug 2016
Many thanks for your comments and suggestions. I see your point and I agree.
I have restructured the the pull request accordingly. See here for the diff:
https://github.com/foswiki/distro/pull/13/files
It differs slightly from Michael's code (s/hidelink/hidefile/, linefeeds). It produces the exact same HTML as before.
--
PhilippeKehl - 16 Aug 2016
George, I hope it's okay that I've now set this to "Needs merge" and "Waiting for" (you).
Thanks!
--
PhilippeKehl - 06 Sep 2016