seyhunak / twitter-bootstrap-rails

Twitter Bootstrap for Rails 6.0, Rails 5 - Rails 4.x Asset Pipeline
https://github.com/seyhunak/twitter-bootstrap-rails
4.5k stars 998 forks source link

Unable to add method to menu_item block #801

Closed patrickgordon closed 9 years ago

patrickgordon commented 9 years ago

Using menu_item as a block & are unable to specify a method (e.g. delete)

Example:

<%= menu_group :pull => :right do %>
    <%= drop_down "#{current_user.full_name}" do %>
        <%= menu_item destroy_user_session_path, method: :delete do %>
            <i class="fa fa-sign-in"></i> Sign out
        <% end %>
    <% end %>
<% end %>

Expected outcome for the is:

<a data-method="delete" href="/users/sign_out" rel="nofollow">
    <i class="fa fa-sign-in"></i>Sign out
</a>

Actual:

<a href="/users/sign_out">
    <i class="fa fa-sign-in"></i> Sign out
</a>
toadkicker commented 9 years ago

Hmm we have a test for this as well.. https://github.com/seyhunak/twitter-bootstrap-rails/blob/master/spec/lib/twitter_bootstrap_rails/navbar_helper_spec.rb#L110, but it isn't rendered in a dropdown. That shouldn't matter though...

patrickgordon commented 9 years ago

Weird... I can provide more info if you need it - just let me know.

toadkicker commented 9 years ago

Would you mind posting your gemfile? I just gave it a try in teststrap and it rendered as expected. I'll push up the code to the develop branch there so you can see it.

patrickgordon commented 9 years ago

Sure

source 'https://rubygems.org'

# Bundle edge Rails instead: gem 'rails', github: 'rails/rails'
gem 'rails', '4.1.5'
# Use postgresql as the database for Active Record
gem 'pg'
# Use SCSS for stylesheets
gem 'sass-rails', '~> 4.0.3'
# Use Uglifier as compressor for JavaScript assets
gem 'uglifier', '>= 1.3.0'
# Use CoffeeScript for .js.coffee assets and views
gem 'coffee-rails', '~> 4.0.0'
# See https://github.com/sstephenson/execjs#readme for more supported runtimes
# gem 'therubyracer',  platforms: :ruby

# UI experience - Twitter bootstrap rails
gem 'therubyracer'
gem 'less-rails' #Sprockets (what Rails 3.1 uses for its asset pipeline) supports LESS
gem 'twitter-bootstrap-rails'

#Google maps
gem 'gmaps4rails'

#Twitter bootstrap 3+ friendly forms
gem 'bootstrap_form'

# Devise for authentication
gem 'devise'

# Easy roles gem to set up roles.
gem 'easy_roles'

# CanCan for authorization
gem 'cancan'

# Login w/ facebook
gem 'omniauth-facebook'

# Carrierwave for image uploading
gem 'carrierwave'
gem 'mini_magick'

# Possessive for things like Patrick's profile
gem "possessive"

#PostGIS
gem 'activerecord-postgis-adapter'

# Use jquery as the JavaScript library
gem 'jquery-rails'
# Turbolinks makes following links in your web application faster. Read more: https://github.com/rails/turbolinks
#gem 'turbolinks'

# To fix turbolinks & js issues
#gem 'jquery-turbolinks'

# Build JSON APIs with ease. Read more: https://github.com/rails/jbuilder
gem 'jbuilder', '~> 2.0'
# bundle exec rake doc:rails generates the API under doc/api.
gem 'sdoc', '~> 0.4.0',          group: :doc

# Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring
gem 'spring',        group: :development

# Use ActiveModel has_secure_password
# gem 'bcrypt', '~> 3.1.7'

# Use unicorn as the app server
# gem 'unicorn'

# Use Capistrano for deployment
# gem 'capistrano-rails', group: :development

# Use debugger
# gem 'debugger', group: [:development, :test]
toadkicker commented 9 years ago

What happens if you comment out bootstrap_form?

patrickgordon commented 9 years ago

Do I need to re- bundle after commenting out?

toadkicker commented 9 years ago

Yes

patrickgordon commented 9 years ago

Negative... issue persists after commenting out bootstrap_form & re-bundling & restarting server.

toadkicker commented 9 years ago

https://github.com/toadkicker/teststrap/tree/develop

method_delete

toadkicker commented 9 years ago

<%= menu_item destroy_user_session_path, method: :delete do %>

to <%= menu_item "Something", destroy_user_session_path, method: :delete do %>

toadkicker commented 9 years ago

The first argument of menu_item expects a string for the text you want to show. If you don't want text to show (though I don't know when this would be the case) you can pass empty string.

patrickgordon commented 9 years ago

I added "Something" and got this: In the navbar:

<%= menu_item "Something", destroy_user_session_path, method: :delete do %>
    <i class="fa fa-sign-in"></i> Sign out
<% end %>
<a data-method="delete" href="Something" rel="nofollow">
    <i class="fa fa-sign-in"></i> Sign out
</a>
toadkicker commented 9 years ago

Just added it to teststrap and got the method: :delete output as expected! So I don't think it is devise.

copy paste:

= menu_group :pull => :right do
   .form-group
     = menu_item 'Log Out', destroy_user_session_path, class: 'btn btn-primary btn-sm', method: :delete
patrickgordon commented 9 years ago

@toadkicker whilst that works it isn't for a block though. I'm using a block because I want to use a font-awesome icon in the button.

toadkicker commented 9 years ago
= menu_item content_tag(:i, '', :class => 'fa fa-refresh') + ' Log Out', destroy_user_session_path, class: 'btn btn-primary btn-sm', method: :delete
patrickgordon commented 9 years ago

Success! That works. However, doesn't explain why the original doesn't work but that's definitely an acceptable work around.

Up to you whether you want to close it or not.

Thanks a lot @toadkicker :+1:

toadkicker commented 9 years ago

Or as a block:

= menu_item destroy_user_session_path, class: 'btn btn-primary btn-sm', method: :delete do
              = content_tag(:i, '', :class => 'fa fa-refresh') + ' Log Out'
toadkicker commented 9 years ago

It's your ticket, if you're happy close it =D

toadkicker commented 9 years ago

Also in my PR for #802 I'm introducing icon helper, so this will be cleaner if you use it.

patrickgordon commented 9 years ago

Yeah I was gonna suggest that / was going to try make it. But looks like you got it covered. Cheers mate for the help.

toadkicker commented 9 years ago

https://github.com/seyhunak/twitter-bootstrap-rails/blob/master/app/helpers/navbar_helper.rb#L20 is calling for name if block_given?, so I believe that is forcing the method to accept name as a first param.