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

Handle edits within frontend fields #7178

Merged
merged 9 commits into from Dec 8, 2022
Merged

Handle edits within frontend fields #7178

merged 9 commits into from Dec 8, 2022

Conversation

jasonvarga
Copy link
Member

@jasonvarga jasonvarga commented Dec 8, 2022

This PR is related to #6400.

Up until now, front-end fields were only used for the "forms" feature where you only ever create items, not edit them. i.e. you only create submissions.

The fields that get rendered would only need to handle default values and old values.

Now with #6400, you'll need to render the fields but also be able to insert existing values.

This PR adds support for existing values.

  • value is the smarter variable that'll account for the value itself, the previously submitted/old value, or default values.
  • The old/default variables are no longer passed to the views, but stay there for backwards compatibility.
  • A bunch of tests are added to make sure appropriate checkboxes are checked, options selected, fields filled, etc.

@what-the-diff
Copy link

what-the-diff bot commented Dec 8, 2022

  • The old variable is now called value.
  • Inline labels are no longer supported, but can be added back in by adding the inline_label attribute to a field's config array and setting it to true or false (defaults to false).
  • Added support for input types other than text on default fields via an optional "input_type" key/value pair in the field's config array (e-mail, password etc.). Defaults to 'text'.
  • Fixed bug where checkboxes were not checked when using multiple values with one being selected as opposed to all of them being selected at once - this was due to incorrect usage of PHP's in_array function which requires two parameters instead of three like Twig does; also fixed similar issue with radio buttons that would cause none of them ever getting checked if there were more than one option available and only one had been previously saved as the correct answer while others weren't even present yet during initial form rendering before any data has been submitted yet; both issues have now been resolved thanks again! :)

@jasonvarga jasonvarga changed the title field rendering Handle edits within frontend fields Dec 8, 2022
@jasonvarga jasonvarga marked this pull request as ready for review December 8, 2022 21:45
Comment on lines +130 to +134
$missing = str_random();
$old = old($field->handle(), $missing);
$default = $field->value() ?? $field->defaultValue();
$value = $old === $missing ? $default : $old;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context for future us, or any onlookers:

This weirdness with the random string old fallback is because of how Laravel's handling of old values works.

If a field is submitted with an empty value, it gets converted to null, so old('fieldname', 'fallback') will give you null. If the field wasn't submitted at all, the fallback would be returned.

If you were to intentionally submit a text field with an empty value, there's no way to tell the difference between the old empty value, or a completely missing value. If we did old('field') ?? $default then we'd end up rendering the default value when a user empties out the field - that's not right.

One option would be to exclude that field from the ConvertEmptyStringsToNulls middleware, but that's overkill and we don't have control of that anyway.

This is the alternative - if the old() method gives us the fallback, we can be sure it's actually missing and not just emptied by the user. I'm using a random string just to prevent someone being able to submit the known string.

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.

None yet

1 participant