Refactor template variable creation

[smg6511]

What does it do?

This proposed enhancement changes the way that TV input and output properties are incorporated into the TV editing form, generating these dynamically-loaded fields as Ext components (as the rest of the form does) instead of a rendered DOM element. This allows a level of manipulation of those fields (and native Ext tracking of their dirty states) that's not currently possible, as well as fully-functional validation.

To preserve compatibility with community-developed custom TVs, the original renders and templates were left untouched and a new directory "configs" was created alongside the original "renders" one. The structure within is essentially the same, though the field configs are set up directly in the php files instead of loading a Smarty-parsed tpl file. These configs are subsequently iterated in javascript and inserted as items into a new fieldset component.

Other changes and enhancements include:

• Default Value and Input Option Values fields are only shown when appropriate (this was fixed ahead of this PR, but is implemented in a different way here).
• Provides a way for user-contributed TVs to hide Default Value and Input Option Values fields (via properties set in the php config, not explicitly illustrated in this PR)
• Default Value field and its labelling changes based on the chosen TV type (e.g., when a file TV is created, its DV field is of the type file and opens the file browser when its trigger is clicked)
• Example code in help descriptions is now subtly highlighted and gets inserted into the relevant field when clicked. Undo functionality is provided by new reset and clear icon buttons (shown next to field label when hovered).
• The actual modx tag for the TV being created now shows in its Name description and gets copied to the clipboard when clicked. The coloring of the clickable text and bg is different than that of the code examples to hint that the expected action is different (copy vs. field insertion).
• Many lexicon entries have been created and/or updated to be more descriptive and precise. Some existing keys were changed to align with the established naming convention ("xxx_desc" for descriptions, "xxx_msg" for messages). I kept references to the originals at the end of each lexicon file edited, just in case these keys are used elsewhere (although I suspect they are not). Note that I wanted to wait to align the non-English language lexicons with this change until it was approved.
• New methods created for form panels to provide a clear path for TV extras (that update to the new loader) to add their own listeners and validators for their field configs to the stock set on the fly (not explicitly illustrated in this PR, but I would create a skeleton example override to show usage).
• Changed date TV input options Start Day and Disabled Days fields to be much more easily usable (should be backward compatible for those who might have saved comma-delimited numbers in the original text fields)
• Created new xcheckboxgroup xtype, used by date Disabled Days, for cb groups that aggregate their checked values into a hidden field (and only save the value of the hidden field to the serialized properties db field). Placed in utilities.js for possible future use elsewhere.
• Minor tweaks to existing SCSS.
• Made many changes to the structure of the field configs to fix layout issues, making all fields stretch to the appropriate widths when the panel's width is changed (also fixes display in mobile context).
• Applied switch/slider style to checkboxes to make consistent with how they appear in Resources. (Note: It probably goes without saying that this would not be done when a checkbox appears in a grid.)
• Groups of fields that only apply in a certain context (e.g., time options for date TV only apply when hideTime is set to "No") only show when appropriate.

Why is it needed?

To lay the groundwork to solve problems with validation (the dirty state of fields and forms, currently implemented in a very non-ideal way) and refine the control over/display of element form fields (particularly that of TVs). Assuming eventual approval of these changes, applicable ones will then be created along the same guidelines for other elements (chunks, snippets, etc.). The advantages will become even more apparent when my validation fixes for 2.x (from PR #15146) are introduced into 3.x (which I'll turn to once this is hopefully merged).

How to test

• First create all of the native TV types and confirm expected functionality, both in the TV editing panel and when created TVs are placed within a test Resource. (Link to a SQL dump of all core TV types).
• Install at least one user-contributed TV extra to confirm that the legacy loading functionality works as expected in the TV editing panel.
• Note the highlighted content of the help descriptions, and click to confirm new insertion feature (clicked example gets inserted into field)
• Confirm that fields with insertion feature show the reset and clear utility buttons when hovering over their labels.

Related issue(s)/PR(s)

Related to #15164, the original draft version of these changes for 2.x.

[JoshuaLuckers]

This is a really huge PR, I left some general feedback on a small part of the changes.

I have not checked whether this PR is technically the best approach.

[smg6511]

Re whether this is the best technical approach: I think you'll find it is at least a much better one. I found when diving into the Ext side of this that its implementation in MODX pretty much crippled its excellent ability to track the dirty state of forms because of the way some components get rendered to the DOM, taking them out of the Ext environment. Their likely was never the need to add/use the markDirty methods currently in MODX's implementation (which end up setting the real dirty state inaccurately most of the time) had a different approach been taken. Note that I say this humbly, not sharply -- I'm sure there were reasons at the time and maybe there's something I'm not taking into consideration, but why not improve this if we can?

[Mark-H]

Sometimes the reason something is the way it is, is nothing more than "a decade ago it seemed like a good idea". ;)

I'm currently trying it out, and I have to say I am impressed with the UI side of it. It's so much more native, and there's a lot of very useful tidbits (like the example values) that greatly improve the user experience setting them up.

Obviously with a PR that adds 5k lines, there is going to be discussion about some of the details, but so far I'm impressed!

--

For others looking to quickly create all TVs, here's a SQL dump of all core TV types. May need to truncate your TV table as the IDs start with 1:, and afterwards edit your template to enable the TVs.

INSERT INTO `modx_site_tmplvars` (`id`, `source`, `property_preprocess`, `type`, `name`, `caption`, `description`, `editor_type`, `category`, `locked`, `elements`, `rank`, `display`, `default_text`, `properties`, `input_properties`, `output_properties`, `static`, `static_file`) VALUES
(1, 1, 0, 'autotag', 'Autotag', '', '', 0, 0, 0, '', 0, 'date', '', 'a:0:{}', 'a:2:{s:10:\"allowBlank\";s:4:\"true\";s:16:\"parent_resources\";s:0:\"\";}', 'a:2:{s:7:\"default\";s:5:\"false\";s:6:\"format\";s:13:\"%a, %b %e, %Y\";}', 0, ''),
(2, 1, 0, 'checkbox', 'Checkbox', '', '', 0, 0, 0, 'White==#ffffff||Black==#000000', 0, 'default', '', 'a:0:{}', 'a:2:{s:10:\"allowBlank\";s:4:\"true\";s:7:\"columns\";s:1:\"1\";}', 'a:0:{}', 0, ''),
(3, 1, 0, 'date', 'Date', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:7:{s:10:\"allowBlank\";s:4:\"true\";s:8:\"startDay\";s:1:\"0\";s:12:\"disabledDays\";s:0:\"\";s:13:\"disabledDates\";s:0:\"\";s:12:\"minDateValue\";s:0:\"\";s:12:\"maxDateValue\";s:0:\"\";s:8:\"hideTime\";s:1:\"1\";}', 'a:0:{}', 0, ''),
(4, 1, 0, 'email', 'Email', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:3:{s:10:\"allowBlank\";s:4:\"true\";s:9:\"minLength\";s:0:\"\";s:9:\"maxLength\";s:0:\"\";}', 'a:0:{}', 0, ''),
(5, 1, 0, 'file', 'File', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:1:{s:10:\"allowBlank\";s:4:\"true\";}', 'a:0:{}', 0, ''),
(6, 1, 0, 'hidden', 'Hidden', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:0:{}', 'a:0:{}', 0, ''),
(7, 1, 0, 'image', 'Image', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:1:{s:10:\"allowBlank\";s:4:\"true\";}', 'a:0:{}', 0, ''),
(8, 1, 0, 'listbox-multiple', 'Listbox multi select', '', '', 0, 0, 0, '@SELECT `pagetitle`,`id` FROM `[[+PREFIX]]site_content` WHERE `published`=1 AND `deleted`=0', 0, 'default', '', 'a:0:{}', 'a:4:{s:10:\"allowBlank\";s:4:\"true\";s:10:\"stackItems\";s:5:\"false\";s:5:\"title\";s:0:\"\";s:9:\"typeAhead\";s:1:\"0\";}', 'a:0:{}', 0, ''),
(9, 1, 0, 'listbox', 'Listbox single select', '', '', 0, 0, 0, 'Bird||Cat||Dog', 0, 'default', '', 'a:0:{}', 'a:3:{s:10:\"allowBlank\";s:4:\"true\";s:5:\"title\";s:0:\"\";s:9:\"typeAhead\";s:1:\"0\";}', 'a:0:{}', 0, ''),
(10, 1, 0, 'number', 'Number', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:7:{s:10:\"allowBlank\";s:4:\"true\";s:13:\"allowNegative\";s:5:\"false\";s:8:\"minValue\";s:0:\"\";s:8:\"maxValue\";s:0:\"\";s:13:\"allowDecimals\";s:1:\"0\";s:16:\"decimalPrecision\";s:1:\"2\";s:16:\"decimalSeparator\";s:1:\".\";}', 'a:0:{}', 0, ''),
(11, 1, 0, 'option', 'Radio', '', '', 0, 0, 0, 'Bird||Cat||Dog', 0, 'default', '', 'a:0:{}', 'a:2:{s:10:\"allowBlank\";s:4:\"true\";s:7:\"columns\";s:1:\"1\";}', 'a:0:{}', 0, ''),
(12, 1, 0, 'resourcelist', 'Resource list', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:5:{s:10:\"allowBlank\";s:4:\"true\";s:9:\"minLength\";s:0:\"\";s:9:\"maxLength\";s:0:\"\";s:5:\"regex\";s:0:\"\";s:9:\"regexText\";s:0:\"\";}', 'a:0:{}', 0, ''),
(13, 1, 0, 'richtext', 'Richtext', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:1:{s:10:\"allowBlank\";s:4:\"true\";}', 'a:0:{}', 0, ''),
(14, 1, 0, 'tag', 'Tag', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:1:{s:10:\"allowBlank\";s:4:\"true\";}', 'a:0:{}', 0, ''),
(15, 1, 0, 'text', 'Text', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:5:{s:10:\"allowBlank\";s:4:\"true\";s:9:\"minLength\";s:0:\"\";s:9:\"maxLength\";s:0:\"\";s:5:\"regex\";s:0:\"\";s:9:\"regexText\";s:0:\"\";}', 'a:0:{}', 0, ''),
(16, 1, 0, 'textarea', 'Textarea', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:1:{s:10:\"allowBlank\";s:4:\"true\";}', 'a:0:{}', 0, ''),
(17, 1, 0, 'url', 'URL', '', '', 0, 0, 0, '', 0, 'default', '', 'a:0:{}', 'a:1:{s:10:\"allowBlank\";s:4:\"true\";}', 'a:0:{}', 0, '');

[Mark-H]

Some feedback from trying this out a little and reviewing the code:

1. UI looks fantastic. Core TVs seem to work as expected. Haven't tried 2.x custom TVs yet, but will try to do that soon. AWESOME WORK!

2. The property files are hand-creating JSON... have you tried using json_encode to ensure it's valid JSON and doesn't break by a stray ' or other special character before landing on this approach?

3. I'm seeing the following copyright notice on top of a lot of files:

 * This file is part of a proposed change to MODX Revolution's tv input option rendering in the back end.
 * Developed by Jim Graham (smg6511), Pixels & Strings, LLC (formerly Spark Media Group)

It's obvious you've put a lot of effort into this and you deserve to be credited for that, however it is not standard to put such notices for anyone in any place in the core and I would ask you to remove (=replace with standard one) because: (1) I worry this "ownership statement" has license implications, even though as you've signed the CLA you have agreed that any changes become ownership of the project; (2) if we accept this, it may set a precedent contributors can go ahead and stamp their name in files they add/change if it's big enough -- that could get messy, and we have the git history and contributors listing for that; (3) I'm hoping we can get this merged for the beta and then suddenly this comment becomes factually incorrect as it is no longer a proposal but the actually merged core ;)

After we get this merged I'm more than happy to talk about a MODX.today post about this change that very clearly and publicly acknowledges the work you've done on making it happen. In terms of getting you/your company some exposure I think that would be both more appropriate and effective.

4. Documentation. I understand you don't want to spend too much time on that before knowing if it's something that'll make it into core, but I think it would help to have (developer) documentation explaining how a custom TV might define the options this way. It's certainly different from the current system, so some documentation stubs of how to make it work would probably.

5. Cleanup A. There's some test methods/console logging that needs to be cleaned up before merge. You're already aware of this as the ones I found so far were marked with a comment to be removed before publishing; just making sure we don't forget.

6. Cleanup B. As this replaces the older properties handling, I was expecting to see those removed (and added to the clean files upgrade script) to avoid confusion, but they're still there in core/src/Revolution/Processors/Element/TemplateVar/Renders/mgr/ - probably something you already were planning to do as well, but making sure we don't forget.

7. Question: output property handling is unchanged, right? The Configs dir is used to define them and provide the UI, but that's still rendered by Renders/web/output, correct?

[smg6511]

Mark - thanks for taking a look so quickly. I know it's a lot to look at. Suffice it to say, once I go down the rabbit hole, I can't help myself! Anyway, to respond to your feedback:

1. Thanks for the positive feedback. You should find the non-native TVs all work as they have, as they call what I refer to as the "legacy loader," which is the current method left untouched.
2. Yes, I've taken into account the JSON encoding. String values are handled and encoded at the top of all the config files. But by all means try to trip it up to find anything I may have missed. I actually found some existing problems re JSON encoding on a few of the input renders (which were fixed in the PR as well).
3. I put this identifying info in knowing that if the proposed change were accepted that it would need to be removed in its present form. I'm not expecting attribution in the files.
4. Yes, documentation is a must and I have that in mind. I will have the perfect example to build from as what motivated this whole concept was the creation of a Time TV, which I can use as a working example and then also create a skeleton example/quick start code to illustrate how to implement TVs in the new loader and apply your own listeners and validators. As you anticipated, I didn't want to put the effort into that until knowing the PR might get incorporated.
5. Yes, there are a handful of places where I thought I might purposely leave example/test code for you to verify some of the new functionality in a most basic sense — seeing that a named listener in fact gets transformed into an actual listener in the Ext config. I can pull this stuff out any time. Just let me know...
6. Again, as I wasn't entirely sure this would fly (mostly because of how big a change it is) I didn't remove the files that would no longer apply. As we refine I can take those out whenever you like.
7. The output properties are changed as well. I have not touched the renders yet; same thing—I wanted to see if the PR was viable before even approaching that end. Also, it's such a major change to how it's been done that it's probably better to make that change in other areas in manageable chunks (i.e., separate PRs over time).

[smg6511]

Note that in this most recent commit, the code quality check is failing due to 2 warnings in one file on line length. The trouble is, breaking those lines up makes them less readable IMO. Should these warnings result in all files failing? Doesn't seem right ... please advise.

RE documentation stubs: What would be the most useful way of providing those at this point in time?

[Ruslan-Aleev]

Great job! Visually, everything works as it should, but there are a couple of comments.

1. Your PR is using outdated .tpl for TV, there are changes that are missing in your PR, for example:

• For TV with the "Number" type, the "allowNegative" parameter has been removed - Removed allowNegative parameter for TV with number type #15119
• For TV with the "Resource List" type, a "typeahead was" added - Add typeahead to Resource List TV #15734
• Added "allowBlank" check for "Input Option Values". Previously, it was possible to create, for example, checkboxes, but not set "Input Option Values" and there was an empty TV in resource form - Changed hiding/showing "Input Option Values" and added allowBlank check #15131

Most likely there are also changes in TVs that are missing in this PR.

2. UX Comments.

• Empty areas appear.

• "Allow Decimals" checkbox does not work correctly.

• The input field "Default value" for some TVs is not full width, although for others it is full width. TV where "Default value" is not full width: Date, Number, Text and Textarea.

Although for TV with an image type and a file, I would, on the contrary, reduce the width of the field "Default value", as it is now in the resource (not 50%, but 400px). Firstly, uniformity in the interface, and secondly, it is less to reach the mouse to the upload icon.

• The "Allow Blank" selection should always be at 100% and not mix with other TV parameters.
  This is a separate important parameter, and it is better to make it easier to find and it would always be in the same place.

In general, in many PRs, I previously tried to standardize the position of TV parameters, it would be great if this work was not in vain :)

Thanks for the PR!

[smg6511]

Quick comments re RA's review:

• You'll need to run grunt build in _build/templates/default to see the updated css. I did not provide the compiled css in the PR, assuming reviewers would want to build the js and css to ensure no issues arose in that area. (An example of what could cause an issue: A contributor directly edits the compiled css but not the scss.) This will fix some of the issues you're seeing.
• I think we're of the same mind in general when it comes to trying to standardize various parts of the UI. However, I'd ask you consider a bit more dedicating so much room to the Allow Blank field. I just think it looks odd and demands an unnecessary amount of real estate. It still always follows in the same order as before and isn't hard to spot. Ideally I think this field should be a switch (checkbox) to the right of the TV type that's labelled "Required?" or something of the like. I get why it's "Allow Blank" (because that matches Ext JS's parameter), but arguably that's the opposite of what most people would expect. I did not propose that (the position change in particular) in my changes because I realized the field structure as it relates to the data (db) structure could be problematic. Anyway, please reconsider before I go through and change this field.

I'll respond to the other items shortly...

[Ruslan-Aleev]

You'll need to run grunt build in _build/templates/default to see the updated css. ... This will fix some of the issues you're seeing.

Yes, I ran grunt build, my comments on the updated styles. Check it out for yourself too.

I think this field should be a switch (checkbox) to the right of the TV type that's labelled "Required?"

Yes, I agree with that.

[smg6511]

On the styles, I see from your screen grabs that the updates are not showing for you. I've double-checked here and it's looking as it should. Take a look at this screen grab (note the checkboxes were updated to the switch style used elsewhere):

Re you comment on the widths of the file and image default fields, I see what you mean if you're editing on a large screen. I'll adjust these by giving them a max-width or give them a special class so the width can be set according to the breakpoints. Hard-coding this in the config with a pixel value is problematic when viewing on mobile (where all fields should stack and be 100% width). At any rate, we're on the same page on this, I'll cook up something to fix it.

[Ruslan-Aleev]

It's strange that the styles haven't been updated, I'll double-check.

[smg6511]

One other quick note for all: Please review this with compress_js off, as there's a fair amount of ES6 in the js code that won't compile correctly with the current build workflow. I have another PR (#15793) to update the workflow that looks like it has a good chance of adoption very soon, probably well before this PR is fully approved. If not, I'll adjust as needed.

[smg6511]

Thinking through ways to address some fields getting too wide on larger screens, maybe we should be limiting the width of the inner panels like shown here (900px in this example):

What do you think?

[smg6511]

Circling back to the Allow Blank placement, I did a bit of playing around with what would need to be done to place it next to the input type as a switch and reversing the logic so it's labelled as "Required?" instead of "Allow Blank." It turned out to not be as tricky as I thought it'd be. Here's a mock up of how that placement would actually look:

If this is preferred by the reviewers, I can commit the necessary changes.

[JoshuaLuckers]

Circling back to the Allow Blank placement, I did a bit of playing around with what would need to be done to place it next to the input type as a switch and reversing the logic so it's labelled as "Required?" instead of "Allow Blank." It turned out to not be as tricky as I thought it'd be. Here's a mock up of how that placement would actually look:

If this is preferred by the reviewers, I can commit the necessary changes.

Since this PR is already pretty big I would suggest to leave it as it is right now.

[JoshuaLuckers]

I really love what you did with this PR, thanks a lot for all the effort you put into this!

A console error is thrown when selecting "Resource List" as Input Type if you try to create a new Template Variable.
SyntaxError: Unexpected token '}'

I guess this is caused by the response from the connector:

{'success': 1, 'optsItems': [
    {
        defaults: {
            layout: 'form',
            labelSeparator: ''
        },
        items: [
            {
                xtype: 'panel',
                columnWidth: 1,
                autoHeight: true,
                labelAlign: 'top',
                defaults: {
                    anchor: '100%',
                    msgTarget: 'under'
                },
                items: [{
                    xtype: 'combo-boolean',
                    fieldLabel: _('required'),
                    description: '',
                    name: 'inopt_allowBlank',
                    hiddenName: 'inopt_allowBlank',
                    id: 'inopt_allowBlank',
                    tabIndex: 1,
                    value: true
                },{
                    xtype: 'label',
                    forId: 'inopt_allowBlank',
                    html: "Select \u201cNo\u201d to make this TV a required field in the Resources it\u2019s assigned to. (Default: \u201cYes\u201d)",
                    cls: 'desc-under'
                }]
            }
        ]
    },
    {
        defaults: {
            layout: 'form',
            labelSeparator: ''
        },
        items: [
            {
                xtype: 'panel',
                columnWidth: 0.5,
                autoHeight: true,
                labelAlign: 'top',
                defaults: {
                    anchor: '100%',
                    msgTarget: 'under'
                },
                items: [{
                    xtype: 'textfield',
                    fieldLabel: _('resourcelist_parents'),
                    description: '',
                    name: 'inopt_parents',
                    id: 'inopt_parents',
                    maskRe: '[0-9,]',
                    value: null
                },{
                    xtype: 'label',
                    forId: 'inopt_parents',
                    html: "If specified, this TV\u2019s listing will include only the child resources from this comma-separated set of resource IDs (containers).",
                    cls: 'desc-under'
                }]
            },{
                xtype: 'panel',
                columnWidth: 0.5,
                autoHeight: true,
                labelAlign: 'top',
                defaults: {
                    anchor: '100%',
                    msgTarget: 'under'
                },
                items: [{
                    xtype: 'combo-boolean',
                    fieldLabel: _('resourcelist_includeparent'),
                    description: '',
                    name: 'inopt_includeParent',
                    hiddenName: 'inopt_includeParent',
                    id: 'inopt_includeParent',
                    value: false
                },{
                    xtype: 'label',
                    forId: 'inopt_includeParent',
                    html: "Select \u201cYes\u201d to include the Resources specified in the Parents field in the list.",
                    cls: 'desc-under'
                }]
            }
        ]
    },
    {
        defaults: {
            layout: 'form',
            labelSeparator: ''
        },
        items: [
            {
                xtype: 'panel',
                columnWidth: 0.333,
                autoHeight: true,
                labelAlign: 'top',
                defaults: {
                    anchor: '100%',
                    msgTarget: 'under'
                },
                items: [{
                    xtype: 'numberfield',
                    fieldLabel: _('resourcelist_limit'),
                    description: '',
                    name: 'inopt_limit',
                    id: 'inopt_limit',
                    allowNegative: false,
                    allowDecimals: false,
                    value: 0
                },{
                    xtype: 'label',
                    forId: 'inopt_limit',
                    html: "The maximum number of Resources shown in this TV\u2019s listing. (Default: 0, meaning unlimited)",
                    cls: 'desc-under'
                }]
            },{
                xtype: 'panel',
                columnWidth: 0.333,
                autoHeight: true,
                labelAlign: 'top',
                defaults: {
                    anchor: '100%',
                    msgTarget: 'under'
                },
                items: [{
                    xtype: 'combo-boolean',
                    fieldLabel: _('resourcelist_limitrelatedcontext'),
                    description: '',
                    name: 'inopt_limitRelatedContext',
                    hiddenName: 'inopt_limitRelatedContext',
                    id: 'inopt_limitRelatedContext',
                    value: false
                },{
                    xtype: 'label',
                    forId: 'inopt_limitRelatedContext',
                    html: "Select \u201cYes\u201d to only include the Resources related to the context of the current Resource.",
                    cls: 'desc-under'
                }]
            },{
                xtype: 'panel',
                columnWidth: 0.334,
                autoHeight: true,
                labelAlign: 'top',
                defaults: {
                    anchor: '100%',
                    msgTarget: 'under'
                },
                items: [{
                    xtype: 'numberfield',
                    fieldLabel: _('resourcelist_depth'),
                    description: '',
                    name: 'inopt_depth',
                    id: 'inopt_depth',
                    allowNegative: false,
                    allowDecimals: false,
                    value: 
                },{
                    xtype: 'label',
                    forId: 'inopt_depth',
                    html: "The number of subfolders to drill down into for this lising\u2019s search query. (Default: 10)",
                    cls: 'desc-under'
                }]
            }
        ]
    },
    {
        defaults: {
            layout: 'form',
            labelSeparator: ''
        },
        items: [
            {
                xtype: 'panel',
                columnWidth: 1,
                autoHeight: true,
                labelAlign: 'top',
                defaults: {
                    anchor: '100%',
                    msgTarget: 'under'
                },
                items: [{
                    xtype: 'textarea',
                    fieldLabel: _('resourcelist_where'),
                    description: '',
                    name: 'inopt_where',
                    id: 'inopt_where',
                    value: null,
                    plugins: new AddFieldUtilities.plugin.Class
                },{
                    xtype: 'label',
                    forId: 'inopt_where',
                    html: "\n    <p>A JSON object of one or more Resource fields to filter this TV\u2019s listing of Resources.<\/p>\n    <div class=\"example-list\">Some examples:\n        <ul>\n            <li><span class=\"example-input\">[{\"template:=\":\"4\"}]<\/span> (only include Resources with template 4 applied)<\/li>\n            <li><span class=\"example-input\">[{\"pagetitle:!=\":\"Home\"}]<\/span> (include all Resources, except for those named \u201cHome\u201d)<\/li>\n            <li><span class=\"example-input\">[{\"class_key:IN\":[\"MODX\\\\Revolution\\\\modWebLink\",\"MODX\\\\Revolution\\\\modSymLink\"]}]<\/span> (include only Resources whose Resource Type is Weblink or Symlink)<\/li>\n            <li><span class=\"example-input\">[{\"published\":1},{\"isfolder\":0}]<\/span> (include only Resources that are published and are not containers)<\/li>\n        <\/ul>\n    <\/div>\n    <p>Note: Filtering by TV values is not supported.<\/p>\n",
                    cls: 'desc-under'
                }]
            }
        ]
    },
    {
        defaults: {
            xtype: 'panel',
            layout: 'form',
            labelAlign: 'top',
            autoHeight: true,
            labelSeparator: ''
        },
        items: [
            {
                id: 'tv-input-props-typeahead-enable',
                cls: 'fs-toggle',
                columnWidth: 1,
                defaults: {
                    anchor: '40%',
                    msgTarget: 'under'
                },
                items: [{
                    xtype: 'xcheckbox',
                    hideLabel: true,
                    boxLabel: _('combo_typeahead'),
                    description: '',
                    name: 'inopt_typeAhead',
                    id: 'inopt_typeAhead',
                    inputValue: 1,
                    checked: false
                },{
                    xtype: 'label',
                    forId: 'inopt_typeAhead',
                    html: "Populate and autoselect options that match as you type after a configurable delay. (Default: No)",
                    cls: 'desc-under toggle-slider-above'
                }]
            }
        ]
    },
    {
        xtype: 'fieldset',
        layout: 'form',
        id: 'tv-input-props-typeahead-options-fs',
        title: 'Type-Ahead Options',
        autoHeight: true,
        columnWidth: 1,
        items: [{
            layout: 'column',
            defaults: {
                xtype: 'panel',
                layout: 'form',
                labelAlign: 'top',
                autoHeight: true,
                labelSeparator: ''
            },
            items: [
                {
                    columnWidth: 0.25,
                    defaults: {
                        anchor: '100%',
                        msgTarget: 'under',
                        hideMode: 'visibility'
                    },
                    items: [{
                        xtype: 'numberfield',
                        fieldLabel: _('combo_typeahead_delay'),
                        description: '',
                        name: 'inopt_typeAheadDelay',
                        id: 'inopt_typeAheadDelay',
                        allowNegative: false,
                        allowDecimals: false,
                        value: 250
                    },{
                        xtype: 'label',
                        forId: 'inopt_typeAheadDelay',
                        html: "Milliseconds before a matched option is shown. (Default: 250)",
                        cls: 'desc-under'
                    }]
                },
                {
                    columnWidth: 0.25,
                    defaults: {
                        anchor: '100%',
                        msgTarget: 'under',
                        hideMode: 'visibility'
                    },
                    items: [{
                        xtype: 'combo-boolean',
                        fieldLabel: _('combo_forceselection'),
                        description: '',
                        itemCls: 'disabled',
                        name: 'inopt_forceSelection',
                        hiddenName: 'inopt_forceSelection',
                        id: 'inopt_forceSelection',
                        disabled: true,
                        value: true
                    },{
                        xtype: 'label',
                        forId: 'inopt_forceSelection',
                        html: "Disabled; only list matches are valid.",
                        cls: 'desc-under'
                    }]
                },
                {
                    columnWidth: 0.5,
                    defaults: {
                        anchor: '100%',
                        msgTarget: 'under',
                        hideMode: 'visibility'
                    },
                    items: [{
                        xtype: 'textfield',
                        fieldLabel: _('combo_listempty_text'),
                        description: '',
                        itemCls: 'disabled',
                        name: 'inopt_listEmptyText',
                        id: 'inopt_listEmptyText',
                        disabled: true,
                        value: ''
                    },{
                        xtype: 'label',
                        forId: 'inopt_listEmptyText',
                        html: "Disabled; selections will always match the list.",
                        cls: 'desc-under'
                    }]
                }
            ]
        }],
        listeners: {
            afterrender: function(cmp) {
                const typeAheadCmp = Ext.getCmp('inopt_typeAhead');
                if (typeAheadCmp) {
                    const   typeAheadOn = typeAheadCmp.getValue(),
                            switchField = 'inopt_typeAhead',
                            toggleFields = [
                                'inopt_typeAheadDelay',
                                'inopt_forceSelection',
                                'inopt_listEmptyText'
                            ]
                            ;
                    typeAheadCmp.on('check', function(){
                        this.toggleFieldVisibility(switchField, cmp.id, toggleFields);
                    }, this);
                    if(!typeAheadOn) {
                        this.toggleFieldVisibility(switchField, cmp.id, toggleFields);
                    }
                }
            },
            scope: Ext.getCmp('modx-panel-tv')
        }
    }
]}

[smg6511]

OK, I think I've covered most of the issues/requests thus far:

Re @JoshuaLuckers' comments above:

• The error in the Resource List TV is fixed
• I had a feeling that holding off on repositioning/naming the Allow Blank might best be left to a subsequent PR. I'll save the code for later.

Re @Ruslan-Aleev's comments:

• Removed "allowNegative" from Number TV
• Fixed Allow Decimals behavior in Number TV
• Typeahead is in my version of the Resource List TV, with the irrelevant fields disabled
• Made input options required for native list-type TVs
• I've held off on changing field widths until discussing a little further; I'm just wanting to get my rationale across beforehand. On this part of the design, I'm trying to take into account not only consistency but visual balance on the page and attempting to keep fields to a more natural size for their intended content. Not to say I'm 100% convinced I've got it all right, just saying that's where I'm coming from.

I also made a minor change to a few of the processors to quiet the error about loading smarty the old way (the rest of the changes in those files are just PSR-12 style fixes).

[Mark-H]

Given it another test with the latest changes, and all seems well!

[Mark-H]

@smg6511 Sorry about my earlier "two weeks" estimate turning out too be too positive. It hadn't fallen off my radar - the radar just started overflowing with other things.

I'm also notoriously bad at timeframes, especially when giving them on a good day before other things start happening ;)

[smg6511]

@Mark-H - No problem, thanks for making the time to look at this again!

@Ruslan-Aleev @Jako @Ibochkarev - If there's any way one or more of you guys can give this a review in the near future I'd really appreciate it. As you all may have seen, @opengeek would like at least 3 approvals to merge this. The sooner this is (hopefully) incorporated, the more time to work out any issues. I'd also love to be able to build upon this by reworking the render functionality (to the resource editing form) well before we get too far into the beta stages, especially since that will be a major reworking as well. Thanks - I know there's a lot going on here trying to move 2.x and 3.x forward, and everyone's got regular jobs and lives and other things going on!

[Jako]

Strange lexicon entries.

[Jako]

This looks like some test description. Is this text used somewhere?

[opengeek]

This has been merged. Can we create an issue for this instead?

[Jako]

This looks like some test description. Is this text used somewhere?

[opengeek]

This has been merged. Can we create an issue for this instead?

[Jako]

The only one who can quick answer this, created the pull request. So it is IMHO better placed here.

[smg6511]

Ooops, good catch ... looks like I missed removing those. Yes, I used your daterange TV in one of my tests for ensuring backward compatibility and for override testing.

[rthrash]

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/proposal-to-move-element-code-into-its-own-tab/4562/1