grav-plugin-form
grav-plugin-form copied to clipboard
Added evaluate option for all non-iterable field types and checkboxes
This PR adds the option to evaluate
all kind of field default values. Currently every string based field and checkboxes are supported. This is very helpful to prefill forms based on query strings for example. This PR is safe against XSS, as only the default value gets evaluated. The evaluation function should be written with care, as always.
It also fixes the hidden field again: https://github.com/getgrav/grav-plugin-form/commit/f61725c9e1f79cf8d414fc2f7a6a62d234cc8715#commitcomment-45792369
Examples:
name:
label: Title
placeholder: 'Enter the new title'
type: text
evaluate: true
default: 'page.title'
validate:
required: true
my_field:
type: checkboxes
label: A couple of checkboxes
evaluate: true
default:
- "{{uri.query('happy') is same as('true') ? 'option2'}}"
options:
option1: Option 1
option2: Option 2
my_field_keys:
type: checkboxes
label: A couple of checkboxes
evaluate: true
default:
option1: `{{uri.query('happy') is same as('true')}}`
options:
option1: Option 1
option2: Option 2
This PR needs a rebase after https://github.com/getgrav/grav-plugin-form/pull/472 is merged. I will take care of that then.
Edit: I'd also like to discuss to make use of evaluate_twig
instead of "only" evaluate
. This gives us more options, and the syntax is used in multiple process
options as well. It would break for hidden fields, but I can add a fix if desired. If we do not add this, more complex functions would need the following workaround, which is quite ugly:
default: "null -}}
{% set test = page.find('/blog/my_first_post').title %}
{{- test -}}
{{- null"
Edit 2: The evaluated twig gets directly injected into the template. Meaning that setting variables with names that have been previously used will cause a lot of trouble. I found, that it makes more sense to create a new page template that inherits the form template with some additional macros that can be used inside the form. This should also improve performance, if the variables are used multiple times in the form.
On the other side i found it way simpler to write: default: "New {{ page.title }}" instead of
default: "'New' ~ page.title". When looking at https://github.com/getgrav/grav-plugin-form/pull/474 it would make sense to keep them consistent. It would make sense to document, that setting variables here is dangerous. Also I dont know how many users will use it combined with setting variables.
==> I've choosen evaluate_twig
now and added backwards compatibility support for the hidden
field.
Edit3: I just noticed, that the default value is set in the form like that. Maybe that should be disabled when using evaluate?
It can be fixed with: https://github.com/getgrav/grav-plugin-form/pull/473/commits/b7167846de8e509f8a2124fd651e85d1d61593df
But what is that field used for?
I would like us to get some standard on namin field features, first. Like I said in https://github.com/getgrav/grav-plugin-form/pull/474#pullrequestreview-565195428, we need some better approach which both indicates the intent better (allowing you to guess what the property does) and allows us to add more similar features without conflicts.
This issue may also affect admin forms, so we need to make sure those keep on working as expected.