piotrmurach / loaf

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

Rails 5.1 has removed before_filter method #9

Closed bananaappletw closed 7 years ago

bananaappletw commented 7 years ago

I am using rails 5.1 and this gem, prompt this error

undefined method `before_filter' for ApplicationController:Class Did you mean? before_action

Maybe there are some code inside this gem using before_filter to lead this error

piotrmurach commented 7 years ago

Hey, thanks for using the library!

The last release of this gem was january 2015 and as is the case with Rails few things have changed since. However, fear nothing! The filter issue has been fixed in the master branch as well as other problems. I'm currently using this gem in rails v5.1.4 by pointing to master branch. I'm planning to develop it further before releasing final version. There are few breaking changes in api that I wish to make based on my current usage but I'm documenting everything in README as I'm going along.

I estimate I should get something ready in next couple of weeks. In the mean time if you can keep using branch version that would be cool.

bananaappletw commented 7 years ago

@piotrmurach Thank you for the work. I just fix some document according current behavior #10 Actually, I found some methods are missing, like clear_breadcrumbs. Maybe there is something wrong inside the code. I could also provide some help.

piotrmurach commented 7 years ago

Thank you for offering to help. The current README is more a look into the future on what the library will provide. For example, the old api:

breadcrumbs do |name, url, styles|
  link_to name, url
end

will change to:

breadcrumb_trail do |crumb|
  link_to crumb.name, crumb.url
end

This has advantage of providing more flexibility for user on how to provide markup and easy way for me to expose more methods on crumb instead of cluttering the yielded parameters. I also don't want people to provide css classes for what is a concern of markup layout rather than ruby library. I hope you like the plans?

bananaappletw commented 7 years ago

@piotrmurach Yes, the new way is better than the old one. Here is the way how I use this gem. I have to calculate the last crumb for specific css class, so I used enum_for(:breadcrumbs) instead of block. I think the initial parameter for breadcrumbs can be removed. Just provide the simple crumbs object, leave the rest for the user.

piotrmurach commented 7 years ago

I've implemented all the changes that intended for the new release. All docs should be up to date - please see breadcrumb_trail for specific details. Do you have time to give it a go in your project before I release new version?

Your snippet should be:

<ul class="ui huge breadcrumb">
  <% breadcrumbs.each do |breadcrumb| %>
    <% if breadcrumb.current? %>
      <%= link_to breadcrumb.name, breadcrumb.path, class: "active section" %>
    <% else %>
      <%= link_to breadcrumb.name, breadcrumb.path, class: "section" %>
      <div class="divider"> / </div>
    <% end %>
  <% end %>
</ul>
bananaappletw commented 7 years ago

@piotrmurach I have tried. But, I encountered some problems, still looking for the solution. For example, I set root 'abouts#show'. When I browse '/' path, I expect breadcrumb acted as Home/About. Instead, HomeAbout/ is shown. I knew the current? helper use URI to match path. Still thinking is there a better solution or compromise for this scenario. What do you think?

piotrmurach commented 7 years ago

I'm not sure what your Home breadcrumb matches to? Because it looks like both the Home & About point to /. Would you mind pasting the code for the breadcrumb invocation and the actual routes definition for both breadcrumbs?

There is a new current_crumb? implementation which is pretty advanced and allows for various matches against urls. But you shouldn't be using it directly as it's internal api used by breadcrumb_trail. Please see new breadcrumb helper parameter :match for the example configuration options.

bananaappletw commented 7 years ago

routes.rb

root 'abouts#show'
resource :about, only: [:show]

application_controller.rb

class ApplicationController < ActionController::Base
  add_breadcrumb 'Home', :root_path
end

abouts_controller.rb

class AboutsController < ApplicationController
  def show
    breadcrumb 'About', about_path
  end
end

Problem is when I visit / about's breadcrumb is included, but that's not the current breadcrumb. Cause the URL is /, the home's breadcrumb is marked as current. It seems like the common scenario that someone will also meet, maybe?

piotrmurach commented 7 years ago

Your routes resolve to the following URIs and controller action combinations:

root 'about#show' -->  '/'  and 'about#show'
resource :about, only: [:show] -->  '/about' and 'about#show' 

Therefore as you can see, in both cases when you access '/' or '/about' URI you will hit the same controller and action. This means that you will get 2 breadcrumbs when accessing these URIs. So when you visit '/' two breadcrumbs will be displayed with the following information (I keep the breadcrumbs data in simple form [name, path, current?] for the example's sake):

['Home', '/', true] => the current path is '/' so the 'Home' breadcrumb is current
['About', '/about', false] => the current path is '/' so the 'About' breadcrumb is not current

It hope its clear now why 'Home' breadcrumb is highlighted when you're on the root path and not the 'About' breadcrumb. So if you desire for the 'About' breadcrumb to be current then you would need to navigated to '/about' URI for it to match. Does that make sense?

bananaappletw commented 7 years ago

@piotrmurach Umm, I understand the code. Anyway, thank you for answer that. However, the block way of breadcrumb_trail doesn't feet my requirement. I use the old way instead, it works fine. I updated the code, you could checkout https://bamboofox.cs.nctu.edu.tw/ and https://github.com/bamboofox/bamboofox-website/blob/master/app/views/layouts/_breadcrumb.html.erb. Thank you for upgrade this gem.