search

Log1x / sage-directives

A set of Blade directives for use with Roots Sage.
https://log1x.github.io/sage-directives-docs/
MIT License
284 stars 35 forks source link

Add @menu, @hasmenu and @endhasmenu #147

Closed davidwebca closed 5 months ago

davidwebca commented 5 months ago

I love using your directives in blade files, but this one was missing for a while and I was initially debating about its utility, but I find myself not using Navi on smaller projects but hate using @php or passing ['echo' => false] in the arguments just to have those ugly exclamation brackets to echo it {!! !!}.

Conversations could be had about having @menu be smarter and do the "has_nav_menu" automatically which is the "normally expected behavior" (the nav location you call doesn't have anything = don't show anything), but in reality WordPress acquainted people could be thrown off by this since wp_nav_menu does some internal checks to always have something output in the navigation when the location has nothing in it like displaying the website's pages when no menu location has been set, thus allowing people to preview themes more easily. And so, I ended up deciding against it to keep this simple, but you be the final judge!

Tested locally on a real project, PHP 8.2, WordPress 6.5.

Thanks for looking at this.

Log1x commented 5 months ago

hey! im cool with this addition. we could maybe just add the has_nav_menu check onto @menu but also keep @hasmenu and @endhasmenu that way you can control container markup as well as use it standalone.

otherwise, just needs tests added to https://github.com/Log1x/sage-directives/blob/master/tests/Unit/WordPressTest.php and a mention in the docs.

edit: ahh re-reading what you said about the rendering – yeah, that makes sense. perhaps just leave the has_nav_menu stuff as-is then. :)

davidwebca commented 5 months ago

Great, here you go, tests added!

Log1x commented 5 months ago

sweet!