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

Add items argument to menus #29

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fahrradflucht
Copy link
Contributor

In some cases the cost of fetching the menus first and then only fetching the items for the menus that are needed by id are higher then fetching them all at once. It's now possible to specify an items argument to indicate whether to fetch the menu items as well or not.

Since it defaults to false this change would be non breaking.

Note that this change is currently V2 only.

As noted in unfulvio#18 it's helpfull to get the slug of the referenced
object in some cases (e.g. custom routing).

Now every item and possible children have an `object_slug` field.
In case of custom links it just returns the items slug since the
id refers to the item it self.

Note that the v1 implementation isn't tested.

Closes unfulvio#18
In some cases the cost of fetching the menus first and then only
fetching the childs for the menus that are needed by id are higher
then fetching them all at once. It's now possible to specify an
`items` argument to indicate whether to fetch the menu items aswell
or not.

This could be also helpfull in cases like [this](https://wordpress.org/support/topic/get-menu-data-by-name-not-id) to filter on the client, although a `slug` argument to filter the menus array in addition would be need.

Note that this change is currently V2 only.
@fahrradflucht
Copy link
Contributor Author

Note that this PR is already based on #28.

@fahrradflucht
Copy link
Contributor Author

fahrradflucht commented Nov 21, 2016

@mckernanin I would merge this but I think it would be great if I could get a second per of eyes before that. Could you give a quick review?

Let me know if you need any additional information or context to make this easier for you.

@mckernanin
Copy link

I will check it out

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.

2 participants