Skip to content

Fix row iterator value desync between data attribute and jQuery data when reordering rows#1518

Merged
jtsternberg merged 2 commits into
CMB2:developfrom
angryaxi:fix-issue-1461
Oct 22, 2024
Merged

Fix row iterator value desync between data attribute and jQuery data when reordering rows#1518
jtsternberg merged 2 commits into
CMB2:developfrom
angryaxi:fix-issue-1461

Conversation

@angryaxi

Copy link
Copy Markdown
Contributor

Description

Motivation and Context

Fixes #1461

The cmb.shiftRows method only updates the row iterator value stored in the data attribute, however the wysiwyg.initRow method that initializes the fields after rearrangement gets the iterator values using the jQuery data method instead.

The changed code now correctly updates both of the iterator values and thus keeps them in sync.

I also separated the code that's responsible for updating the iterator in data attributes from the one that updates the name attributes. Both of the iterator values are now updated a bit further down the code and it signals more clearly that the values should be kept in sync for developers making changes to the method in the future.

Risk Level

I think the risk level of this change is quite small as it only affects the editor.

Testing procedure

I have tested that ordering groups containing wysiwyg fields works correctly in both the classic editor and the block editor.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

@jtsternberg jtsternberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks good, thank you!

@jtsternberg jtsternberg merged commit 9567dd6 into CMB2:develop Oct 22, 2024
@RubenMartins

Copy link
Copy Markdown
Contributor

This looks good, thank you!

Hi @jtsternberg following this merged pr can we expect an release with this fix any time soon?

@RubenMartins

Copy link
Copy Markdown
Contributor

hi @jtsternberg sorry to chase, but do you have an estimated time to release this fix?

@Quintrillium

Copy link
Copy Markdown

+1 for release date of this fix. Thanks.

@marianadates

Copy link
Copy Markdown

Also looking for a release date of this fix

@angryaxi angryaxi deleted the fix-issue-1461 branch April 15, 2025 09:56
@angryaxi

Copy link
Copy Markdown
Contributor Author

I'd also like to see this change being incorporated into a proper release.

Could @tw2113 take a look at this, since he is listed as one of the maintainers for the cmb2 package on Packagist?

@tw2113

tw2113 commented Apr 15, 2025

Copy link
Copy Markdown
Contributor

I help with general support/communications, but I still leave anything commit/release with Justin.

@matiast89

Copy link
Copy Markdown

Hi, when will this fix be incorporated? The bug that this would fix has caused a lot of issues. Thanks.

@angryaxi

Copy link
Copy Markdown
Contributor Author

Hi @jtsternberg are there any plans to make a new release with this fix included? A bit over a year has passed since this PR was merged. Let me know if there's anything I could do to help.

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.

Reordering wysiwyg fields inside a group causes a problem with TinyMCE

7 participants