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

Fix Replicator/Grid/Bard not displaying newly added items after saving #7000

Merged
merged 4 commits into from
Nov 4, 2022

Conversation

jasonvarga
Copy link
Member

@jasonvarga jasonvarga commented Nov 3, 2022

Fixes #6993

The reported issue was about Replicator, but it also happened in Bard and Grid.

Previously, we would use ids to keep track of reordering etc just in JS, and throw the IDs away when you save just so you wouldn't get them in your data.

Now that we update the form with fresh values since #6842, the fieldtype preprocessing would assign new IDs, the JS would get confused, and the newly added items would mismatch, cause a JS error, and not display.

There would be no data loss, it would just look like it.

This PR will keep the IDs in the data/yaml. It's the simplest solution we could find, and there's no harm to having id keys in there.

Todo:

  • Put id at the top of the array. It's just a little nicer, it's awkward at the end especially if there are nested arrays.
  • See how nicely this plays with sites using https://github.com/aryehraber/statamic-uuid to store their own id fields. It will now be redundant.

@what-the-diff
Copy link

what-the-diff bot commented Nov 3, 2022

  • Replaced uniqid() with str_random(8) in BardFieldtype.vue
  • Replaced uniqid() with str_random(8) in Replicator.vue
  • Added a mock for the RowId facade.
  • Replaced hardcoded row IDs with random strings generated by the mocked RowId facade in preprocessed values and processed values (in tests).

@jasonvarga
Copy link
Member Author

jasonvarga commented Nov 3, 2022

This seems to have defeated what-the-diff.

After pushing more commits, the comment now kicks ass again. (Not perfect, but still pretty incredible)

@aerni
Copy link
Contributor

aerni commented Jan 2, 2023

@jasonvarga Does this change make the way for granular localization as discussed in statamic/ideas#439?

@jasonvarga
Copy link
Member Author

Not intentionally, but yes it would make it easier.

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.

Since 3.3.50, replicator loses fields on save
2 participants