Write a object oriented UI library

[Bloke]

It's high time to phase out most/all of txplib_forms.php and parts of txplib_html.php. To achieve this, we should build an extensible OO user interface library using inheritance and interfaces.

Benefits:

• Removal of the stupid overloaded parameter function signatures (like fInput() with 12 params!) in favour of clean constructors with setters.
• Additional attributes can be catered - e.g. data- attributes.
• Cascading and compound UI elements can be built more easily from the base components - in core and in plugins - reducing code.
• Potential to construct one or more tags that output elements, which allows forms to be built.
• Potential to use the tags to build the admin-side UI from templates, which offers endless workflow customization options or easily buildable dashboards for people that know what they're doing.

Downsides:

• It's OO, so possibly slower than raw function calls. Needs to be optimised to maintain speed.
• Bigger product footprint overall because each discrete file has preamble text.

So far:

Class	Notes	Proposed names
AdminAction	Replaces eInput(), sInput(), tInput()	-
Checkbox	A single <input type="checkbox" /> tag	-
CheckboxSet	An <input type="checkbox" /> tag inline set with labels	-
Disclosure	A twistable panel behind an anchor, which replaces our current wrapGroup and wrapRegion	-
Form	A <form /> tag	-
Input	An <input /> tag	-
InputSet	An <input /> tag set. Primarily of use for creating a series of hidden inputs	-
Label	A single <label /> tag	-
OnOffRadioSet	An <input type="radio" /> tag set with on/off options	-
OptGroup	An <optgroup /> tag and collection of options	-
Option	An <option /> tag	-
Radio	A single <input type="radio" /> tag	-
RadioSet	An <input type="radio" /> tag inline set with labels	-
Script	A <script /> tag	-
Select	A <select /> list tag	-
SelectTree	A <select /> list tag built from a tree-based record set	-
Tag	Base widget - a tag for creating custom widgets	-
TagCollection	Useful for creating a custom collection of widgets	-
Textarea	A <textarea /> tag	-
Token	A hidden <input /> tag containing a CSRF token	-
YesNoRadioSet	An <input type="radio" /> tag set with yes/no options	-

[bloatware]

+1, sure, please start with upload_form(:-). The speed is not critical on the admin side, imo, and the extra footprint is justified if it helps to avoid code redundancy.

[philwareham]

I'm in favour of this massively. Maybe out could all justify the name Textpattern 5!? If we coincided it with unlimited custom fields of various types too that seems enough to warrant a new numbered release.

[philwareham]

Would you be considering adopting a PHP framework as part of this?

[bloatware]

I would rather investigate on the side of template languages (like mustache), to be able to process forms both server and client side.

[philwareham]

Sure. As long as the basic structure of HTML isn't a million miles away from what we currently have and is easy to style, then it gets my vote.

[Bloke]

Processing client and server forms did occur to me, but I never got my head round mustache fully. Seemed useful though. I've used Twig, fwiw but found the control structures lacking.

What I'm proposing here - at least initially - is just a way to maintain our notion of "checkbox", "radio set", "select input", "textbox" and so forth, along with their granular counterpart "tag", and their combined versions such as "file input", "checkbox set", "upload form", etc and just wrap them in a better, extensible manner. Like tag lego. We already have most of the components in place, it's just that they're:

a) functions
b) overloaded with parameters
c) not very flexible to change (requires introduction of new params)

So I was planning on simply writing a few quick objects to replicate the existing functionality in a more OO style, and make it easier to customize (and shorter to invoke than counting which positional argument to use when you need to specify an id!)

Longer term, the objects could be invoked from a tag (e.g. <txp:widget type="upload" name="file" class="file-upload" multiple required />) and you'd get a form field spat out. Value validation might be part of all this too (patterns), along with checking you've supplied 'mandatory' attributes (such as name) so things don't break.

Things this is NOT:

• A raw HTML building framework (although technically the Tag object could be used to build any tag or structure, it'd drive you mad to do so).
• A templating language, as such. It's just a series of shortcut objects for outputting common form controls or building your own, from smaller components (such as tag, checkbox, label, etc).
• Bloated. It should adopt sensible defaults so that most widgets require minimal customization, but the option to do so should be available.
• A breaking change. It's a new parallel system that core could adopt over time to render admin-side widgets, plugins and admin themes could use, and eventually, will be a complete replacement for txplib_forms.php.

Yes, I know, Not-Invented-Here. I've tried countless widget builders over the years and every one has either left me cold, been too limited, too big, or too complex trying to do everything. This isn't one of those systems.

I've made a start in a separate local branch, but my OO is flaky so it'll take a few iterations to figure out what extends what and interfaces what else. If the scope needs to expand to do more, and it makes sense to do so, let's hear proposals.

[bloatware]

I was planning on simply writing a few quick objects to replicate the existing functionality in a more OO style, and make it easier to customize

That will be already a huge improvement, and if, one day, we are able to compose a whole admin pane from txp tags, my 10-years dream would come true.

Templating is probably off-topic here, but let me explain what I mean by example. Suppose we want to allow theme authors to construct their own upload image previews, with few extra fields (name, resize, thumbnails dimensions). Some of these fields (resize) could be pre-filled on the server side, from user preferences or last used values. Other fields (name) are only available to js. So we need a template language (even our own) that can be parsed both by php and js, and easy to use for theme authors. Something in the vein of

<div>
    <input name='title' value='{title}' />
    <span>{type} ({size} B)</span>
</div>

[Bloke]

That will be already a huge improvement, and if, one day, we are able to compose a whole admin pane from txp tags, my 10-years dream would come true.

Yours and mine too :-)

Re: templating, we should definitely build in some kind of support for this. I don't know how yet. At the moment I have the following Objects:

• Tag - the basic thing that replaces tag(), tag_start(), tag_end(), and tag_void(). Create one of these, setAtts() on it as an array of name-value pairs with optional flags (that can be passed to join_atts()) and call render() with an option of outputting the full tag with content, a self-closer, a start tag, an end tag, or verbatim if nothing matches. If you omit the option (for example, if you just echo the object via its __toString() magic method) then the most appropriate option is used. If you used setContent(), then you get a full tag with content, for example.
• Attribute - one or more attributes in the tag. Attributes can be grouped by properties (such as flags) so that when they are joined via join_atts() the appropriate flags are associated with the group. This permits things like in fInput() where all atts except value are governed by TEXTPATTERN_STRIP_EMPTY. Haven't decided if an Attribute should be just one thing and we have an AttributeCollection for many. My head says a collection is the 'right' thing to do, but my heart and performance practicalities are leaning towards a single object that contains an array of attributes.
• WidgetInterface - defines a few core methods that the tags must define. Not sure I've got this bit right.
• (a test object) Textbox - to replicate fInput() functionality. But does it extend Tag? or is Tag the most specific component and the other objects that are made up of the smaller parts the main players?

Thoughts and ideas welcome.

[Bloke]

Story so far, via examples:

Textboxes

Render a bog standard input box with the given name and value. Note you can just echo or print the tag to output it directly.

$tag = Txp::get('\Textpattern\Widget\Textbox', 'fish', 'cod');

print $tag;

Renders:
<input type="text" name="fish" value="cod" />

You can supply attributes via the setAtts() method, and the value param is optional. The setBool() method is a shortcut for setting one or more boolean attributes:

$tag = Txp::get('\Textpattern\Widget\Textbox', 'valueless')
->setAtts(array('data-whatever' => 'my-custom-attribute'))
->setAtts(array('i-am-boolen' => true))
->setBool(array(
   'required',
   'some-future-boolean-option'
))
->setBool('autocomplete');

print $tag;

Renders:
<input type="text" name="valueless" data-whatever="my-custom-attribute" value="" i-am-boolen required some-future-boolean-option autocomplete />

DIY tags

If you want to delve off-piste, you can go to the lowest level with the Tag class and build your own stuff:

$label = Txp::get('\Textpattern\Widget\Tag', 'label')
->setAtts(array('for' => 'terms'))
->setContent('I agree to the terms');

$tag = Txp::get('\Textpattern\Widget\Tag', 'input')
->setAtts(array(
   'type' => 'checkbox',
   'class' => 't-and-c',
   'id' => 'terms',
   'value' => 1));

echo $tag . $label;

Renders:
<input type="checkbox" class="t-and-c" id="terms" value="1" />
<label for="terms">I agree to the terms</label>

Separate start / end

Custom content? No problem. Here's a form with two fields inside it. Note that during rendering - using the render() method directly instead of the magic methods - we tell it to render the start of the form only, then the two input widgets, then the end of the form.

Also, the action attribute would normally be swallowed by the default 'strip empty' behaviour so we've modifed the flag for that attribute to permit empties. The same thing happened in the Textbox above for the value attribute by default, to permit the valueless attribute.

$mail = Txp::get('\Textpattern\Widget\Textbox', 'email')
->setAtts(array('type' => 'email'));

$area = Txp::get('\Textpattern\Widget\Tag', 'textarea')
->setAtts(array('placeholder' => "Tell me something I don't know"));

$form = Txp::get('\Textpattern\Widget\Tag', 'form')
->setAtts(array('name' => 'sendy', 'id' => 'my-form'))
->setAtts(
   array('action' => ''),
   array('flag' => TEXTPATTERN_STRIP_NONE)
);

echo
   $form->render('start'),
   $mail->render(),
   $area->render('full'),
   $form->render('end');

Renders:
<form name="sendy" id="my-form" action="">
<input type="email" name="email" value="" />
<textarea placeholder="Tell me something I don&#039;t know"></textarea>
</form>

Sky's the limit.

Next up: a collection of objects in the same vein as Textbox that act as convenience wrappers around the Tag so you don't have to specify so many attributes to build common widgets. Hello checkbox, radioset, textarea, selects, ...

Questions or comments / improvements on implementation welcome.

If anyone wants to join the party, see 0fda690 for 'widgets' branch and current code. I don't have world class OO skillz, so anyone who can do this better in a more extensible or adaptable manner, please dive in.

[Bloke]

5c06fea introduces the TagCollection class which, as its name suggests, holds a bunch of tags. This is primarily of use for creating groups of related tags, and the example implementation bundled with the commit is for RadioSet. Essentially, it's an array that holds a set of widgets, in the order you add them to the object. So in the case of RadioSet if you give it an array of 3 key=>label pairs, it'll create:

• Radio 1
• Label 1
• Radio 2
• Label 2
• Radio 3
• Label 3

And when you print/render it, they're just concatenated in that order.

Below is an example of it in use. It's not much different to its functional counterpart. Note that there is no (direct) implementation of setting the id, as it's handled internally via concatenation of the key and value. In the old function-based version, there was an optional id parameter which was added as a prefix to the key-value pair. I'm not sure if there's an actual use case for this: radio sets need to have a unique name anyway, so why prepend an extra value? Anyone any idea whether a manually-set ID prefix is of any practical purpose here?

Note also that the 'no' value is checked in this example by setting the third parameter $default during construction. We should maybe add the ability to set the default value via a method too (e.g. setDefault('some-value')), though I can't think of a reason why it'd be useful right now.

$radset = array(
   'yes' => gTxt('yes'),
   'no' => gTxt('no'),
   'maybe' => gTxt('maybe'),
);

$radioset = Txp::get('\Textpattern\Widget\RadioSet', 'decisionRadio', $radset, 'no');
echo $radioset;

Renders:
<input type="radio" name="decisionRadio" id="decisionRadio-yes" class="radio" value="yes" />
<label for="decisionRadio-yes">Yes</label>
<input checked type="radio" name="decisionRadio" id="decisionRadio-no" class="radio active" value="no" />
<label for="decisionRadio-no">No</label>
<input type="radio" name="decisionRadio" id="decisionRadio-maybe" class="radio" value="maybe" />
<label for="decisionRadio-maybe">maybe</label>

The convention is (as before) that if you set a value as default (checked), it gets the class active appended to it, and the class name itself is named after the widget (radio in this case - and this always makes me smile when I see it active). If you don't like that, you can build your own collection.

Or, after building the stock collection, you can use the getKeys() and getWidget() methods to access the individual widgets and alter their properties by hand prior to rendering. We could probably add an Iterator to the Collection for even greater flexibility so you could use foreach() to step through its members.

The TagCollection concept should be extensible to select lists and checkbox sets, as well as widget combos. I'm hoping this means we can introduce a sensible InputLabel as a convenience method, which will combine the given input widget with a label, and be able to raise a callback as we do now.

I've also introduced a dedicated Label convenience tag, which takes the for attribute as its second argument. It's used internally by RadioSet but you could create arbitrary label tags, with or without the for.

The main advantage over the old way of doing things is, because all tags are based on the Tag object, you can easily addAtts() to any of them to provide custom attributes, without having to overload constructors with huge parameter chains. Keeps things nice and simple, with constructors only providing the bare functionality that you can then build on as needed.

Please take it for a spin and suggest improvements or add to the library if you wish.

[philwareham]

Just a minor thing: can any widgets that are generated have their attributes in the following order at the start of a HTML tag:

1. class
2. id
3. name
4. type

That makes it quicker for me to theme things, as the hooks I need are at the front of the tag so I can view them quickly. Basically, class should be the very first attribute on a tag as that is generally the one we hook to. Cheers!

[Bloke]

Now that I don't know about quite yet!

I did spot yesterday that it seems to be putting them in an undesirable order. It seems to depend on how the object itself is created and which attributes are added first. It seems that booleans always go last. But if I addAtts() a data attribute right at the end, it appears first in the list - but always after name which seems to trump everything.

From what I can tell so far:

• added attributes appear in the order they are added.
• booleans always occur last, in the order they are added.
• name always appears first.

It's on my list of things to investigate, unless someone else wants to step in and hazard a guess why it's doing this.

[Bloke]

Attribute order should be fixed in be6559e. At least, for core-defined helper widgets as far as we can. For custom built ones, it'll render the attributes in the order given.

EDIT: If the core doesn't define any class or id attributes though, they'll get put after the core ones such as name. Not sure how to get round that.

[jools-r]

I like the way this is going!!

[Bloke]

810cf75 adds select list support. Examples:

$optset = array(
   'banana' => 'Banana',
   'strawberry' => 'Strawberry',
   'vanilla' => 'Vanilla',
   'chocolate' => 'Chocolate',
);

$select = Txp::get('\Textpattern\Widget\Select', 'flavour_love', $optset, 'banana, chocolate');

echo $select->render();

Renders:
<select name="flavour_love[]" multiple>
<option value="banana" selected dir="auto">Banana</option>
<option value="strawberry" dir="auto">Strawberry</option>
<option value="vanilla" dir="auto">Vanilla</option>
<option value="chocolate" selected dir="auto">Chocolate</option>
</select>

Note that because you supplied more than one default value, the select was automatically converted to a multiple select. You can always use setMultiple() or build the select manually if you prefer:

$select2 = Txp::get('\Textpattern\Widget\Select', 'flavour_love')
->addOption('banana', 'Banana')
->addOption('pineapple', 'Pineapple')
->addOption('strawberry', 'Strawberry', true)
->addOption('chocolate', 'Chocolate')
->setBool('required')
->setAtts(array('class' => 'selectClass', 'data-whatnot' => 'some value'))
->blankFirst();

echo $select2->render();

Renders:
<select name="flavour_love" class="selectClass" data-whatnot="some value" required>
<option value="">&#160;</option>
<option value="banana" dir="auto">Banana</option>
<option value="pineapple" dir="auto">Pineapple</option>
<option value="strawberry" selected dir="auto">Strawberry</option>
<option value="chocolate" dir="auto">Chocolate</option>
</select>

[Bloke]

I should mention 3c03989 here, which introduces something we don't currently have in our arsenal: OptGroups. Here's an example if you want to directly build a grouped select list:

$mcu = Txp::get('\Textpattern\Widget\Select', 'marvel_characters')
->addOptGroup('Heroes')
->addOption('spiderman', 'Spiderman')
->addOption('iron-man', 'Iron Man', true)
->addOption('thor', 'Thor')
->addOption('professor-x', 'Professor X')
->addOptGroup('Villains')
->addOption('loki', 'Loki')
->addOption('magneto', 'Magneto')
->addOption('ultron', 'Ultron')
->addOption('doctor-doom', 'Doctor Doom')
->setBool('required')
->setAtts(array('class' => 'marvelClass', 'data-mcu' => 'Marvel Cinematic Universe'))
->addOption('black-widow', 'Black Widow', false, 'Heroes');
echo $mcu;

Renders:

<select name="marvel_characters" class="marvelClass" data-mcu="Marvel Cinematic Universe" required>
<optgroup label="Heroes" dir="auto">
<option value="spiderman" dir="auto">Spiderman</option>
<option value="iron-man" selected dir="auto">Iron Man</option>
<option value="thor" dir="auto">Thor</option>
<option value="professor-x" dir="auto">Professor X</option>
<option value="black-widow" dir="auto">Black Widow</option>
</optgroup>
<optgroup label="Villains" dir="auto">
<option value="loki" dir="auto">Loki</option>
<option value="magneto" dir="auto">Magneto</option>
<option value="ultron" dir="auto">Ultron</option>
<option value="doctor-doom" dir="auto">Doctor Doom</option>
</optgroup>
</select>

Note we accidentally forgot Black Widow (the shame of it!), and added her afterwards by specifying the group to add her to. Under normal operation, when it encounters an addOptgroup() it sets it as the 'current' group so any options added after it appear in that group. This permits simple linear construction of option groups, while the extra parameter allows you to go back and add stuff to a specific group at a later date.

Alternatively, you can build your groups separately and add them as content to a select. This gives you more control over each group as you can set attributes directly on each, disable groups, and so forth. Here we add a class and data- attribute to each group:

$mcu2 = Txp::get('\Textpattern\Widget\Select', 'marvel_characters');
$optGroup1 = Txp::get('\Textpattern\Widget\OptGroup', 'Heroes')
->addOption('spiderman', 'Spiderman')
->addOption('iron-man', 'Iron Man', true)
->addOption('thor', 'Thor')
->addOption('professor-x', 'Professor X')
->addOption('black-widow', 'Black Widow')
->setAtts(array('class' => 'marvelClass', 'data-mcu' => 'Marvel Heroes'));
$optGroup2 = Txp::get('\Textpattern\Widget\OptGroup', 'Villains')
->addOption('loki', 'Loki')
->addOption('magneto', 'Magneto')
->addOption('ultron', 'Ultron')
->addOption('doctor-doom', 'Doctor Doom')
->setAtts(array('class' => 'marvelClass', 'data-mcu' => 'Marvel Villains'));
$mcu2->setContent($optGroup1.$optGroup2);
echo $mcu2;

Renders:

<select name="marvel_characters">
<optgroup label="Heroes" class="marvelClass" data-mcu="Marvel Heroes">
<option value="spiderman" dir="auto">Spiderman</option>
<option value="iron-man" selected dir="auto">Iron Man</option>
<option value="thor" dir="auto">Thor</option>
<option value="professor-x" dir="auto">Professor X</option>
<option value="black-widow" dir="auto">Black Widow</option>
</optgroup>
<optgroup label="Villains" class="marvelClass" data-mcu="Marvel Villains">
<option value="loki" dir="auto">Loki</option>
<option value="magneto" dir="auto">Magneto</option>
<option value="ultron" dir="auto">Ultron</option>
<option value="doctor-doom" dir="auto">Doctor Doom</option>
</optgroup>
</select>

I'm deliberating over creating a 'shortcut' method to add options without needing to specify the value, since it can either be derived from the label (dumbed down) or used directly in most cases. Not sure how best to name things, so it might not appear. But in the meantime, this optgroup capability is fairly handy.

[philwareham]

Minor thing, but Textbox seems close to textarea to me, but actually renders an input. I'd rather see Input and Textarea as the names.

[Bloke]

I'd rather see Input and Textarea as the names.

Makes sense. Since everything's an 'input' I was just trying to be more specific. Let's review the names and come up with a table of what we think makes sense for the various input controls.

[philwareham]

Feel free to edit/change/add to the below. I'm thinking since there are many <input> types that we refrain from making it too granular apart from for checkboxes and radios which have a more limited and unique subset of attributes allowed on them. A type attribute could be used to stop it getting too granular.

Select
Textarea
Input
InputRadio
InputRadioSet
InputCheckbox
InputCheckboxSet

[Bloke]

Okay, that's a pretty good suggestion. See first post where I've just put the current implementation list of helper functions. If you want to add the proposals there we can see what makes sense all in one place.

Essentially, every tag is constructed from either the Tag or TagCollection so if you want to make bare bones tags or customise anything to the Nth degree, go there. Otherwise, use the 'helper' methods that bundle up the Tag/TagCollection calls into convenient-to-swallow bundles that cover the most useful options. Just saves typing, that's all.

[Bloke]

Also, if there are any convenience helpers missing that would make people's lives easier when creating tags, feel free to add them to the list (tagged as 'unimplemented' or something) and we can look to add them. Basically, I was just going through txplib_html and txplib_forms and extracting all the useful functions we have today. Might have missed some or not finished some.

[philwareham]

OK, I've added a couple more, and suggested name changes to Textbox as discussed. Hope that is clear.

[Bloke]

Renamed Textbox and TextboxSet as Input and InputSet, respectively.

Is everyone okay with calling this a 'Widget' library? I just chose the name as that's what I call them - input widgets - but if it's better in industry parlance to call them something else, then let's rename the class now.

Maybe UI? Then we'd use:

Txp::get('\Textpattern\UI\Input', 'name', 'value');

Is that better? Anything else we could/should use instead?

EDIT: This also impacts methods such as addWidget() - what would we call those? addElement()? addUI()? ...?

EDIT2: And, longer term, this may affect any public-side tag implementation:

<txp:widget type="upload" name="file" class="file-upload" multiple required /> ?
<txp:ui type="upload" name="file" class="file-upload" multiple required /> ?

[philwareham]

The other concern I have with using switches is that common UX expectations for them these days is that they instantly save a pref when switched. Our prefs panel doesn't do that (you have to then 'save' the prefs). By using a switch we risk user confusion unless you start AJAX-saving the prefs - which may be undesirable.

[Bloke]

That is a valid point re: immediacy.

If we go the Ajax save route then it gives nobody a way of backing out changes. And it'd have to work like that for all prefs - including the admin theme switcher, the site name/URL and comments on/off (which changes the list in the left upon Save, as does toggling Advanced prefs) and so forth.

That also means no Save button, which potentially renders a bunch of plugins that utilise the prefs panel useless, as they won't necessarily be wired for Ajax saves (it requires a change of type for Type 3 plugins to Type 4, at minimum).

[Bloke]

In terms of code it does mean some other weirdness. Anything checkboxy cascades from the UI\Checkbox class. Anything Radioish cascades from the UI\Radio class. So if we add the txp-switch class to those by default, then all radios (including three/four/more choices) and checkbox sets (not that we use them yet in core) would get these classes.

If we move the enforced txp-switch class to just those classes that offer 'binary' choices - UI\YesNoRadio and UI\OnOffRadio - then if you decide to use the bog standard UI\RadioSet and pass it only two options, you won't get the txp-switch by default. Same with checkboxes. That's inconsistent, because the difference between a checkbox toggle and a two-choice radio is usually just down to labelling.

The simplest solution here is to simply add the styles to the admin themes' CSS for handling them, then add the class when we build the controls on an as-needed basis. So the UI library stays pure and we just manually ->setAtt('class', 'txp-form-field-value txp-switch') whenever we construct toggles we know need to behave this way. That's more hassle/code for us but is way more flexible.

[philwareham]

So the UI library stays pure and we just manually...

OK, that works for me - so I can do the (probably optional) CSS for this, how would the following current HTML be changed by your recommendation (feel free to edit the code below)?:

<div class="txp-form-field-value">
    <input class="radio active" id="example1" type="radio" checked>
    <label for="example1">No</label>
    <input class="radio" id="example2" type="radio">
    <label for="example1">Yes</label>
</div>

[Bloke]

One thing we don't have in core right now but could really do with are numeric inputs and range sliders. These bring some unique challenges to a UI library because they have some optional components:

• min
• max
• step

The library itself can of course manage these if you build a UI\Input control by hand and pass in the relevant params:

use \Textptattern\UI\Input;

$nb = new Input('rating');
$nb->setAtts(array(
    'type' => 'number',
    'min' => 1,
    'max' => 10
));

echo $nb;

That would output:

<input name="rating" type="number" min="1" max="10">

But there are some interesting caveats here. Namely we can't use '0' as a value without overriding the default flag behaviour to 'strip empty values':

$nb = new Input('howmuch');
$nb->setAtts(array(
    'type' => 'number',
    'min' => 0,
    'max' => 1,
    'step' => '0.1'
), array(
    'flag' => TEXTPATTERN_STRIP_NONE,
));
echo $nb;

The same thing applies to range sliders.

Should we introduce a Number class that can handle these in a simpler fashion that would automatically apply the correct stripping flag? e.g.

$nb = new Textpattern\UI\Number('donation_amount', 6, 0, 50, 2);

with the last three values being min, max, step: these could be amalgamated into a single array parameter instead so the signature is:

Number($name, $val, $constraints = array('min' => 0, 'max' => '', 'step' => 1));

I kinda like that, but maybe someone has a better idea? It might not be worth going as far as creating a Range class, since it's identical under the hood to a Number: it just changes the type which is easy enough to do with a single attribute. Although we could add one for completeness as it's easy to extend the Number class with a single line of code in a dedicated Range class.

From a usage standpoint, we have another issue with these input types: where to store the sizing info. Let's consider prefs. If we want to expose a numerical input field for, let's say SMTP Port, the value can be (ostensibly) from 1 to 65535. So it makes sense to have a number input with min=1 and max=65535. Same could be said for any numeric inputs we have that are currently using text_input (logs_expire_days, max_url_length, etc).

The problem is that in the txp_prefs table we have no way of specifying these constraints. We have columns for the pref name, its value, its group ('event') and input type (html), visibility, position and scope (private/global).

We could really do with some additional database fields for specifying things like min, max, step, pattern, size and so forth. But not all of them are applicable to all field types so we'll probably need to make some design/storage compromises.

One obvious solution is to introduce a new size column. This means that for text_input types we can specify the small/medium/large/xlarge sizes directly in the pref value if we wish, instead of having to hard-code if we want to deviate from 'medium' in the panel code. I like that. We could perhaps reuse that field for numeric prefs too, e.g. allow a comma-separated list of sizes: min, max, step. Any missing values get defaults.

Not sure what 'size' would mean for YesNoRadio or OnOffRadio prefs. Guess we just have to take the hit on the wasted storage space. Multi-select lists could use 'size' to dictate how many items to display in the list before a scrollbar is shown, perhaps. Textareas could specify rows, cols - is that something we need or does CSS offer everything here?

Do we go further and introduce a pattern column too? That's really only applicable to freeform entry fields so would be wasted for numeric prefs and select lists and, well, pretty much any other type. But that would allow us to offer in-browser validation and also to dynamically build a PrefConstraint when prefs are saved, which would validate the value on the server-side before storage using the same pattern.

That'd be a boon for us (and links to issue #1600 where we wish to validate prefs before saving them to avoid bad values being stored). And it's a boon for plugin authors who want to store prefs as they can employ the same validation rules and have core do the hard work of enforcing their rules by comparing stored values to the pattern and kicking up a fuss if it doesn't match.

I'm just not sure if introducing a column specifically for 'pattern' is too restrictive or a waste of DB storage space. For numeric types, the min/max/step is actually a storage constraint as well as a pattern-matching constraint AND a browser validation constraint, even though the actual HTML input controls don't support pattern explicitly for these types of input.

Since we don't really need both size and pattern for most types, can we just introduce a single additional column in txp_prefs that can cover both? Maybe call it constraint and then whatever is given in that column is dependent on the input control. By default, Radio buttons would validate to 0 or 1 unless you overrode it (somehow). Numbers validate to min, max, step. Input controls could validate to an arbitrary pattern constraint, but we'd have no way of specifying the input control size (width), short of adding an attribute manually when it's built, which means hard-coding it in the prefs panel code.

Alternatively - and very very dirtily - we could let the constraint column accept some key=>value or JSON data that specifies exactly which things we want to validate. e.g. for the image_dir pref, the DB fields could contain:

val: ""
type: 0
event: admin
html: text_input
position: 40
constraint: {
   "size": "medium",
   "pattern": "[a-zA-Z0-9\-\_]"
}

That's not very clean and purists would run a mile at the abuse of data storage "one field = one value" conventions, but it does offer us superior flexibility to handle any and all size/validation opportunities on both browser and server sides in a single compact field.

I'm torn. What do y'all think? Paging @bloatware @philwareham @petecooper @jools-r for comment.

[petecooper]

I'm afraid I won't be much help with this, given the paged audience I will respectfully defer to others.

[Bloke]

No probs. Thanks for looking anyway.

It's one of those conundrums of do we do it "right" (which involves a lot of tables and joins and slowdown) or do we do it "wrong" (which is way more flexible but yucky and less intuitive for plugin authors) or do we leave it in code (which is harder to maintain and not as useful to us or plugin authors) or some hybrid of the above/something else.

None of them are attractive, but since we're building a UI library that is, by its nature, going to be the cornerstone of building our interfaces, it pays to do it in the best way for the project to make the leanest code and least hassle for people using it, regardless of rightness and wrongness.

[philwareham]

I’m for the way that benefits us best, so if two key values (or more) were in the constraints field and that gave us maximum flexibility without hard coding then that seems ok to me.

[Bloke]

Yeah, I'm leaning that way as it does mean we can extend this in really neat ways in future to help our UI features. I just fear the rolleyes from the 4NF crowd who stare agape at our database structure and storm the castle with searing pitchforks for crimes against database design.

[bloatware]

I'd report it here: setting a cf render to checkboxSet (in custom-fields branch) and trying to access Write tab yields

 Uncaught Error: Call to undefined method Textpattern\UI\CheckboxSet::setAtt() in C:\xampp\htdocs\textpattern\textpattern\vendors\Textpattern\Meta\Field.php:862

[bloatware]

Another issue: <input type="datetime" /> used in dateTime render is obsoleted in favour of <input type="datetime-local" />.

[Bloke]

Fixed the first one. Note to self: TagCollections don't have directly addressable IDs

[Bloke]

Another issue: <input type="datetime" /> used in dateTime render is obsoleted in favour of <input type="datetime-local" />.

Surely this can just be changed without any b/c issues anywhere we render such a widget?

EDIT: Where do we render these in core? I can't find one...

EDIT 2: Or do we just have to add datetime-local to the supported types here:

textpattern/textpattern/vendors/Textpattern/Meta/Field.php

Line 881 in ecb0a57

case 'dateTime':

?

[Bloke]

This might fix it: 0e9e37d
Please test.

[bloatware]

Nope, it still outputs type="datetime" for me. This value has been dropped about 8 years ago, so we should be safe replacing it with datetime-local (looks well-supported now).

[Bloke]

Ah balls yeah, the UI library short circuits it by using strtolower(). Hang on.

[Bloke]

Hmm, not very clean but since dateTime is what we refer to a date-time widget internally, I've fudged it in the renderer. Is that any closer?

EDIT: and yeah yeah, braces round the ternary test. Old habits die hard, sorry. Must get out of the habit of doing that but it's a holdover from when that type of construct could cause weird logic errors if the brackets weren't in place. The parser has improved now so they're not needed and I need to move with the times.

[bloatware]

This has nailed it, thank you. Should we also switch to dateTime render in core datetime inputs (posted, expires etc) for consistency?

[bloatware]

Sorry @Bloke, when creating a date/time cf I get this:

Fatal error: Uncaught mysqli_sql_exception: Table 'txpdev.txp_meta_value_datetime-local' doesn't exist in C:\xampp\htdocs\textpattern\textpattern\lib\txplib_db.php:442

[phiw13]

PHP 8.2.11 / MySQL 8, on a fresh CF-branch install
While trying to create a dateTime field:

Fatal error: Uncaught mysqli_sql_exception: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '-local DEFAULT NULL, UNIQUE KEY meta_content (me' at line 5 in /Users/username/Sites/_txp-dev/textpattern/lib/txplib_db.php:442 
Stack trace: 
#0 /Users/username/Sites/_txp-dev/textpattern/lib/txplib_db.php(442): mysqli_query(Object(mysqli), 'CREATE TABLE IF...', 0) 
#1 /Users/username/Sites/_txp-dev/textpattern/lib/txplib_db.php(864): safe_query('CREATE TABLE IF...', false) 
#2 /Users/username/Sites/_txp-dev/textpattern/vendors/Textpattern/Meta/Field.php(389): safe_create('txp_meta_value_...', 'meta_id int(12)...') 
#3 /Users/username/Sites/_txp-dev/textpattern/include/txp_meta.php(587): Textpattern\Meta\Field->save(Array) 
#4 /Users/username/Sites/_txp-dev/textpattern/include/txp_meta.php(553): meta_save() 
#5 /Users/username/Sites/_txp-dev/textpattern/include/txp_meta.php(70): meta_save_ui() 
#6 /Users/username/Sites/_txp-dev/textpattern/index.php(238): include('/Users/username/S...') 
#7 {main} thrown in /Users/username/Sites/_txp-dev/textpattern/lib/txplib_db.php on line 442

–^–

And while playing, I noticed that several CF input fields have an invalid size attribute: input type = date | time | number | range (datetime-local is probably affected too).