Skip to content

Potential fix for fm_renumber and radios bug#658

Merged
dlh01 merged 3 commits intomasterfrom
hotfix/548/radios-sortable-bug
Dec 24, 2020
Merged

Potential fix for fm_renumber and radios bug#658
dlh01 merged 3 commits intomasterfrom
hotfix/548/radios-sortable-bug

Conversation

@mboynes
Copy link
Copy Markdown
Contributor

@mboynes mboynes commented Oct 12, 2017

A proposed solution to the bug reported in #548, this code modifies fm_renumber() by adding a temporary and unique prefix to all modified field names during the renumbering process, then removes that prefix after renumbering is done.

I'm not sold on this approach, it feels like a lot of overhead.

Closes #548

@mboynes mboynes added this to the 1.2.0 milestone Oct 12, 2017
@dlh01
Copy link
Copy Markdown
Member

dlh01 commented Oct 12, 2017

The _.uniqueId() function in Underscore is intended for "temporary DOM ids," which seems similar to what you're going for? http://underscorejs.org/docs/underscore.html#section-164

While that doesn't really reduce the "overhead" of the change (and might even increase the overhead because we need to make sure Underscore loads before fieldmanager.js, although #625 might have the same effect), it might allow us to offload some of the work to an existing library, and it might at least "feel" cleaner.

@mgburns
Copy link
Copy Markdown

mgburns commented Nov 21, 2017

Confirming that this branch does fix the issue for us! 🥇

astratagem pushed a commit to spiritedmedia/wordpress-fieldmanager that referenced this pull request Jul 31, 2018
…o-renumber

[Upstream PR alleyinteractive#658] Potential fix for fm_renumber and radios bug
@astratagem
Copy link
Copy Markdown
Contributor

It works for us too! Thank you.

@kaitlinbolling
Copy link
Copy Markdown

This fix worked for me! Can we get this merged?

@dlh01 dlh01 self-assigned this Oct 23, 2020
@dlh01
Copy link
Copy Markdown
Member

dlh01 commented Dec 24, 2020

While it might be more overhead than ideal in theory (or might not), in practice there are many testimonials to it working, and no new approaches have been presented, so I think we can move ahead. We can always improve its performance in future PRs.

Copy link
Copy Markdown
Member

@dlh01 dlh01 left a comment

Choose a reason for hiding this comment

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

🌳

@dlh01 dlh01 merged commit dd346eb into master Dec 24, 2020
@dlh01 dlh01 deleted the hotfix/548/radios-sortable-bug branch December 24, 2020 04:34
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.

Radio values disappear upon dragging repeating sortable field group

5 participants