Fixed duplicate id issue with secondary nav menu

girishpanchal30 commented 5 months ago

Summary

When the page load, I count the number of secondary menus and add it to the nav menu wrap ID. e.g: secondary-menu_2 ref: https://github.com/Codeinwp/neve/blob/master/.storybook/dummy-data.js#L514

Test instructions

Create Wordpress site with Neve.
Add a header element to both desktop and mobile view (for example a secondary menu)
Save
The HTML source will contain duplicate id values (e.g. secondary-menu)

Check before Pull Request is ready:

[x] I have run all tests and they pass
[x] The code passes when running the PHP CodeSniffer
[x] Code meets WordPress Coding Standards for PHP, HTML, CSS and JS
[x] Security and Sanitization requirements have been followed
[x] I have assigned a reviewer or two to review this PR (if you're not sure who to assign, we can do this step for you)

Closes #4256

pirate-bot commented 5 months ago

Plugin build for 5e821df0a0f4b8ef2da60bbfba2dc7829ad35fe4 is ready :bellhop_bell:!

Download build

girishpanchal30 commented 5 months ago

@preda-bogdan As per your comment I've created unique ID based components, please review my latest commit and let me know if need any changes.

rodica-andronache commented 5 months ago

@girishpanchal30 it's not working for me, let me know if I'm missing something.

This is how I added the secondary menu on desktop https://vertis.d.pr/i/xPdUSS , on mobile https://vertis.d.pr/i/tAcdor and the ID is duplicated https://vertis.d.pr/i/etW95L

girishpanchal30 commented 4 months ago

@rodica-andronache It seems we need to add the device name in the ID to create a unique ID based on components and devices. Please check with the latest build zip.

rodica-andronache commented 4 months ago

@girishpanchal30 thank you! It works well now 👍

pirate-bot commented 4 months ago

:tada: This PR is included in version 3.8.10 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

Codeinwp / neve