Customizer Menus: Creating/saving a new menu creates duplicate menus in Custom Menu widget select

[tywayne]

Steps to reproduce:

1. Create a new menu in the Customizer
2. Save & Publish
3. Navigate to Widgets in Customizer
4. Add a Custom Menu widget
5. Dropdown for menu selection will include duplicates of menus

gif walkthrough of those steps here: ​https://cloudup.com/cxN8AnPZx3j

[celloexpressions]

Reproduced. Probably a reasonably simple fix one someone finds where the code is.

[adamsilverstein]

Remove duplicate menu item insert

[adamsilverstein]

first step for adding menus on the fly to custom menu widgets

[adamsilverstein]

In 32800.diff​:

• Remove a single line that is inserting the duplicate options into the widget template - this fixes the issue mentioned on the ticket
• This patch is ready to go and can be committed.

32800.2.diff​ is an incomplete attempt to insert newly created menus to the custom menu widget template on the fly. With this patch, if you a new menu (without saving), then switch to the widgets and insert the custom menu widget, the new menu will be available in the dropdown. However, the menu id is the placeholderId, and needs updating during save/publish. Since this a different issue, we can probably commit 32800.diff and create a new ticket to address this issue.

[adamsilverstein]

Opened #32814 to cover the enhancement covered in 32800.2.diff. 32800.diff is ready to go and solves the issue on this ticket.

[stevenkword]

Replying to adamsilverstein:

In 32800.diff​:

• Remove a single line that is inserting the duplicate options into the widget template - this fixes the issue mentioned on the ticket
• This patch is ready to go and can be committed.

Patch applies, but I'm seeing a new issue as a result. Following the same reproduction steps WITHOUT Save & Publish, the Custom Menu Widget is not aware of newly created Menus.

Steps to reproduce:

1.) Create a new menu in the Customizer
2.) Navigate to Widgets in Customizer
3.) Add a Custom Menu widget

"No menus have been created yet. Create some."

[tywayne]

Replying to adamsilverstein:

Opened #32814 to cover the enhancement covered in 32800.2.diff. 32800.diff is ready to go and solves the issue on this ticket.

Patch does not fix the issue completely. It does keep it from being added to new Custom Menu Widgets, but if you already had a Custom Menu widget in the Widget Area, it still appends a duplicate. - ​https://cloudup.com/ca0VyaOMdcZ

[adamsilverstein]

I see this issue as well, and working to address it in #32814. We need to use the temporary id until the user saves, then swap that with the real ID on save and publish.

Replying to stevenkword:

Replying to adamsilverstein:

In 32800.diff​:

• Remove a single line that is inserting the duplicate options into the widget template - this fixes the issue mentioned on the ticket
• This patch is ready to go and can be committed.

Patch applies, but I'm seeing a new issue as a result. Following the same reproduction steps WITHOUT Save & Publish, the Custom Menu Widget is not aware of newly created Menus.

Steps to reproduce:

1.) Create a new menu in the Customizer
2.) Navigate to Widgets in Customizer
3.) Add a Custom Menu widget

"No menus have been created yet. Create some."

[adamsilverstein]

Hmmm, thought I tested that. I will revisit and try to fix.

So new steps to reproduce, right?

1. Create a new menu in the Customizer
2. Save & Publish
3. Navigate to Widgets in Customizer
4. Open an existing Custom Menu widget
5. Dropdown for menu selection will include duplicates of menus

Replying to tywayne:

Replying to adamsilverstein:

Opened #32814 to cover the enhancement covered in 32800.2.diff. 32800.diff is ready to go and solves the issue on this ticket.

Patch does not fix the issue completely. It does keep it from being added to new Custom Menu Widgets, but if you already had a Custom Menu widget in the Widget Area, it still appends a duplicate. - ​https://cloudup.com/ca0VyaOMdcZ

[adamsilverstein]

@tywayne To clarify: did you test with 32800.diff​? Thats the one meant to resolve this issue, the .2 patch was the start of further work and can be safely ignored. I'll retest with the first patch.

[tywayne]

Replying to adamsilverstein:

@tywayne To clarify: did you test with 32800.diff​? Thats the one meant to resolve this issue, the .2 patch was the start of further work and can be safely ignored. I'll retest with the first patch.

Correct, tested with 32800.diff​ - patch resolves issue for newly added Custom Menu widgets, but still duplicates on existing widgets for me.

[adamsilverstein]

Replying to tywayne:

Replying to adamsilverstein:

@tywayne To clarify: did you test with 32800.diff​? Thats the one meant to resolve this issue, the .2 patch was the start of further work and can be safely ignored. I'll retest with the first patch.

Correct, tested with 32800.diff​ - patch resolves issue for newly added Custom Menu widgets, but still duplicates on existing widgets for me.

Thanks for the feedback and testing.

This should be fixed in 32800.3.diff​, can you please retest?

The remaining issue I see is that if you create a new menu and DON'T save and publish, then try to select it in a custom menu widget, the selection is not saved correctly. Working on that issue in #32814.

[tywayne]

Tested and confirmed that .3 fixes the duplication bug presented in this ticket.

I'll watch and help test #32814 as needed as well. Looks like there is a @todo at the bottom of api.Menus.applySavedData that could be removed with a fix of #32814.

[jorbin]

@westonruter @ adamsilverstein Is this blocked by #32814 or should it go in now?

[westonruter]

@jorbin I think this is bound up with #32814, yes. The patch in its current form here will not have the desired results if the user had selected the new nav menu in the Custom Menu widget, since the menu gets removed without regard to whether it was selected or not.

[celloexpressions]

Should we close this in favor of #32814, which seems to encapsulate this fix, or keep them separate?

[adamsilverstein]

Replying to celloexpressions:

Should we close this in favor of #32814, which seems to encapsulate this fix, or keep them separate?

I think you can close as the fix on #32814 (when completed)‚ will include this fix.

[westonruter]

Duplicate of #32814.