search

OleVik / grav-plugin-static-generator

Indexing and static generation of Page(s) for Grav.
MIT License
22 stars 1 forks source link

Fix target selectors in the admin settings form #1

Closed klonfish closed 4 years ago

klonfish commented 4 years ago

The target for index and content can't be set using the admin interface, since the validation causes any selection to be stored as true instead of the actual value in the YAML file. This can be observed in the settings form as none of the options is selected after saving.

Removing the validation fixes this issue. This is also the way it is done in the core blueprints (cf. e.g. system.yaml:1252)

OleVik commented 4 years ago

Indeed, I've noticed this, and have fixed it locally. This is the result of replicating YAML too fast and not checking the result; they both should be validated as required: true, and not be type: toggle, but rather type: select. If you want to remedy that for v1.*, I'd be happy to accept the PR and merge.

In v2.0.0 both will be strings, and not rely on the preset selections.

klonfish commented 4 years ago

Initially, I also intended to use select fields, but since I found examples of other three-way toggle fields, I deemed it okay. Additionally, in my opinion it looks a bit odd with required: true, as these two fields are now the only ones with a big red star. Strictly speaking, it is of course correct to set them as required, even though it doesn't seem to be possible to set them empty in the admin form.

Nevertheless, I pushed a new commit with the suggested changes. Since you are working on it anyway, it is probably not that important how it currently looks as long as it works.