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

Suggest crumb.last? #41

Closed
dfreerksen opened this issue Apr 1, 2020 · 3 comments
Closed

Suggest crumb.last? #41

dfreerksen opened this issue Apr 1, 2020 · 3 comments

Comments

@dfreerksen
Copy link

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

  • OS version: macOS Catalina (10.15.4)
  • Ruby version: 2.6.1
  • Loaf version: 0.9.0

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
Copy link
Owner

piotrmurach commented Apr 14, 2020

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
Copy link

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
Copy link

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?

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

No branches or pull requests

3 participants