search

piotrmurach / loaf

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

Suggest crumb.last? #41

Closed dfreerksen closed 4 years ago

dfreerksen commented 4 years ago

Are you in the right place?

Yes

Describe the problem

When you first jump in to loaf and start working with it, crumb.current? isn't very self-explanatory. You would expect crumb.current? to be true for the last breadcrumb. That's not really the case because it is working with the :match condition.

Steps to reproduce the problem

n/a

Actual behavior

Works as expects but with a suggested improvement

Expected behavior

Expected crumb.current? to be true for the last breadcrumb. Had to read into what :match was doing.

Describe your environment

Suggested action

I suggest crumb.last? to be added to the items in the breadcrumb_trail object. The last breadcrumb would set crumb.last? to true. All other objects would be set to false.

piotrmurach commented 4 years ago

Hi David,

Thanks for posting this issue. The history of this gem is long and convoluted, but it held up pretty well over the decade I used it. Unfortunately, I will have to respectfully decline your request to add a new API method. This is what the current? is for. I wish to keep things minimal. To achieve the behaviour you want the :exclusive match is probably what you want.

To make things less confusing, especially to newcomers to the gem, I would be inclined to change the default matching to :exclusive. Alternatively, I'd be happy to review PR with an implementation that configures the gem to skip matching and mark last crumb as current. Do you have time to submit PR?

richardonrails commented 3 years ago

Based on the README example for Bootstrap, and no other documentation about what current? is, I also assumed it refers to the last item. Is that not the case? It'd be good to document what current? actually is then, because I can't tell.

richardonrails commented 3 years ago

I see that you're referencing match (https://github.com/piotrmurach/loaf#213-match) -- does that mean you're trying to auto-detect the visitor's current page? In my understanding of breadcrumbs, the last breadcrumb would always represent the user's page and no URL matching detection would be necessary. When does that assumption breakdown?