Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suffix tricky field names in Blueprint and Fieldset builders #5084

Merged

Conversation

arthurperton
Copy link
Contributor

This suffixes fields names like assets, date and link so they become assets_field, date_field and link_field to avoid problems like #3687.

@jasonvarga
Copy link
Member

Why don't we just do this for all fields?

@jackmcdade
Copy link
Member

Sure, we could do it for all of em – I just like some of the default field names (like text, markdown, etc) – not a big deal. Consistency is probably a better experience.

@arthurperton
Copy link
Contributor Author

So to be clear I'm just slapping a __('Field') at the end of it. That word is in the translation files. Hope the word order works in all languages 🤞🏻

@jasonvarga
Copy link
Member

It's just a placeholder. I don't think the word order matters too much there. 👍

Like I mentioned once before, I think that most of the time if you're using the default placeholder field name as the real field name, more than likely you're just slamming fields into a blueprint quickly to reproduce a bug. If you're building a real blueprint you'd probably give the field name more than a muscle memory's worth of thought. And then like Jack said, consistency is good so only adding "Field" to 3 fields might be weirder.

Addressing issues like #3687 is probably more important.

Just my opinion though!

@arthurperton
Copy link
Contributor Author

I agree. So this is a good step towards preventing issues like #3687. At least by this PR you're not set off on the wrong foot anymore by 'dangerous' examples. We could also consider adding assets to the list of reserved words in the docs and Statamic\Http\Controllers\CP\Fields\FieldsController->blueprint().

@jasonvarga jasonvarga merged commit 154aab9 into statamic:3.2 Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants