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

array keys and item id values no longer match in generated array #66

Closed
daverobertson opened this issue Aug 17, 2023 · 4 comments · Fixed by #72
Closed

array keys and item id values no longer match in generated array #66

daverobertson opened this issue Aug 17, 2023 · 4 comments · Fixed by #72

Comments

@daverobertson
Copy link

Hi,

In the readme's example output, the array's keys and "id" values match, and correspond to the ID of the menu item created in WP. In the example, it starts at 5, 6, etc.

Since (I think) 2.0.4, the keys and id values don't match. For example, the generated array now seems to increments from 1, 2, etc.. which doesn't correspond to the IDs of the menu items created in WP.

This broke a few implementations for us, since we'd set array items based on the menu item ID (e.g. if a particular menu item should be active in certain contexts).

A quick workaround restores the previous behavior:

// generate the array like usual:
$nav = (new Navi())->build('primary')->toArray();

// workaround: set array keys to match menu item ids:
foreach ($nav as $id => $item) {
    $nav[$item->id] = $item;
    unset($nav[$id]);
}

Thanks!

@Log1x
Copy link
Owner

Log1x commented Aug 17, 2023

I did change that as it was an oversight on my part to not have them properly ordered to begin with.

Previously, ordering items in the backend had the potential to be in the wrong order when built by Navi so it is now using the order value as the key since you can already get the ID from the item its self.

I'm sorry I didn't make that more clear as a potential breaking change in the release notes. 😦

@daverobertson
Copy link
Author

Got it!

We've never had an issue with Navi mis-ordering things before, but I understand.

Since this is by design, we'll hang on to our workaround since we do need to modify things based on the menu item's ID. This way the user can move their menu items around. Perhaps it can help others...

Cheers,

@Log1x
Copy link
Owner

Log1x commented Aug 17, 2023

I might've been over-thinking it when I did the change and there was perhaps no issue with having the ID's as it was before since they will always be unique menu ID's. 🤔

I'm not opposed to reverting the behavior – but is there any reason your implementation cant just use $value->id instead of using the key?

@daverobertson
Copy link
Author

Yes; it just makes it easier to update an item when we can reference a unique ID / key. I'm not sure how to do that via $value->id other than looping through the array to evaluate each menu item?

Here's a simplified example of our use case. Sometimes we want to highlight a menu item based on a certain criteria (custom post type, template, special pages, etc):

// $nav is already set by this point.

if (some_condition()) {
    $nav[MENU_ID_ABOUT]->activeAncestor = true;
}

That key could be a constant defined somewhere (as seen here), or a variable set via a custom UI. If the menu item ID is 42, we can reference it directly and easily manipulate the $nav array.

Thanks for considering! Let me know if I can supply any more info,

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 a pull request may close this issue.

2 participants