piotrmurach / loaf

Manages and displays breadcrumb trails in Rails app - lean & mean.
MIT License
407 stars 24 forks source link

Dealing with recursive breadcrumbs #23

Closed brendon closed 6 years ago

brendon commented 6 years ago

Hi @piotrmurach :) I was wondering if you'd given any thought to recursive breadcrumb generation? I've wracking my brain trying to think of a way to do this. In my system there are static breadcrumbs that I can add to controllers and views as usual. But above these 'modules' is a folder-like structure that is basically recursive in nature so I need to create recursive breadcrumbs before finally terminating at the root breadcrumb.

Gretel supports this but it approaches breadcrumbs in a very different manner, though I do wonder if it's possible to have a recursive_breadcrumb method or a breadcrumbs do end approach?

Any ideas would be greatly appreciated :D

piotrmurach commented 6 years ago

Unfortunately, I haven't had the need for this functionality. However, I'm open to ideas. In the past, I was thinking of adding some api to allow for infinitely nested breadcrumbs. I'm not overly ethusied about the recursive_breadcrumb or breadcrumbs names as the former explains the algorithm and the latter is a call used internally and also clashes with twitter bootstrap gem if my memory servers me right. I do like the idea of nested blocks like:

breadcrumb 'posts', :posts_path do
  breadcrumb 'comments', comments_path do
    breadcrumb 'comment', posts_comment_path
  end
end

This should work for both the class level and instance level breadcrumb calls. Any thoughts?

brendon commented 6 years ago

I just realised my use-case could be simply achieved by just doing the following:

before_action do
  folder.ancestors.each do |ancestor|
    breadcrumb ancestor.name, [:admin, ancestor, :contents]
  end
end

So no need to clutter up loaf for that! But working on the PR did help me get to the right conclusion.

I'm having trouble understanding the use-case for your example above. Would you be able to give me a bit more info on why you'd do this vs just flat breadcrumbs?

I do have another idea for recursive breadcrumbs though:

breadcrumbs_for -> { folder.ancestors }, -> (ancestor) { ancestor.name }, -> (ancestor) { [:admin, ancestor, :contents] }

I assume we'd allow it to accept either a Proc that returns an array of items, a symbol to call at runtime to get the items, or just the array of items. Similarly the name and url could be defined as a Proc or just something static, though I'm not sure of the use of a static value in this example.

Thoughts?

piotrmurach commented 6 years ago

I was going down the route of syntactic sugar rather than any 'real' need, that's probably why I decided to never implement it in the first place! I do prefer your first solution, the second one starts to look cryptic, or too 'clever' for its own good. My preference is to keep api to the very minimum unless things cannot be done with the current implementation. If you have time, I would encourage you to contribute to the README and explain your example in case other people will have a similar need. Saves time for everybody.

brendon commented 6 years ago

Thanks @piotrmurach, having played with the before_action technique a bit more, I agree, it's the most flexible way to do this with minimal fuss. An iterator just opens itself up to a whole lot of edge cases and functionality add-ons so best not to go there :) Once I've settled my re-do of breadcrumbs in my app I'll be sure to do a PR with some README updates.