Timber: (feature request) More hierarchical menu properties: Timber\MenuItem::$parent and Timber\Menu::$flat_items

Created on 21 Aug 2018  路  4Comments  路  Source: timber/timber

Expected behavior

  1. The MenuItem class should have a parent field so you can traverse up the hierarchy.
  2. The Menu class could also implement a $flat_items array so people can loop over all the menuitems in a menu without having to use recursion or a stack array.

Actual behavior

  1. The MenuItem class does not have a parent property.
  2. The Menu class does not provide flat_items property, a list of all the menu items it contains, only the direct children.

Most helpful comment

This is a great idea @rubenvincenten.

I do feel pretty strongly that these should be methods and not properties. With parent it's pretty straightforward as a property, but with flat_items that logic to initialize the array has to live somewhere. Making flat_items a property relegates the logic to the constructor or to init(). That starts to feel like treating init() as a junk drawer: it's bad for extensibility, modularity, and testing in isolation. I'd feel much better about flat_items at least getting it's own method.

All 4 comments

I'm using the following: https://gist.github.com/rubenvincenten/ac9ccf53861a06721c26e88b3b855924

I need this because when you have a page 5 levels deep, and a menu only has two levels, then you might want to select one of the ancestors. The code in the gist does that dynamicly but the step to get all the items is just obnoxious if you ask me...

One could also decide to use MenuItem::$menu that refers to the actual Timber\Menu object. Although, $menu is not maybe not descriptive enough...

@rubenvincenten Sounds like a good idea. Would you be up to creating a pull request for this?

This is a great idea @rubenvincenten.

I do feel pretty strongly that these should be methods and not properties. With parent it's pretty straightforward as a property, but with flat_items that logic to initialize the array has to live somewhere. Making flat_items a property relegates the logic to the constructor or to init(). That starts to feel like treating init() as a junk drawer: it's bad for extensibility, modularity, and testing in isolation. I'd feel much better about flat_items at least getting it's own method.

Was this page helpful?
0 / 5 - 0 ratings