techiferous / tabulous

Easy tabbed navigation for Rails
Other
322 stars 35 forks source link

Devise Sign Out Link #20

Closed samzhao closed 11 years ago

samzhao commented 11 years ago

Hi,

Awesome gem! Saves me a lot of trouble manually configuring bootstrap nav items. However, I spent way too long figuring out how to add a sign out link for Devise. Since the sign out helper for Devise is destroy_user_session_path and it needs a method: :delete instead of the default GET action, I need somewhere to put specify the http action for the link in Tabulous. It doesn't appear anywhere in the documentation, which is understandable, and I can't seem to turn the array inside of the config.tabs block into hashes like this:

config.tabs do
    {
      #----------------------------------------------------------------------------------------------#
      #    TAB NAME     |    DISPLAY TEXT    |    PATH         |    VISIBLE?        |    ENABLED?    #
      #----------------------------------------------------------------------------------------------#
      home_tab:    { text: 'Home'     , path: root_path                  , visible: true                                           ,    enabled: true },
      signin_tab:  { text: 'Sign in'  , path: new_user_session_path      , visible: !user_signed_in?                               ,    enabled: true },
      signup_tab:  { text: 'Sign up'  , path: new_user_registration_path , visible: !user_signed_in?                               ,    enabled: true },
      signout_tab: { text: 'Sign out' , path: destroy_user_session_path  , visible: user_signed_in?                                ,    enabled: true },
      admin_tab:   { text: 'Admin'    , path: root_path                  , visible: current_user && current_user.has_role?(:admin) ,    enabled: true }
      #----------------------------------------------------------------------------------------------#
      #    TAB NAME     |    DISPLAY TEXT    |    PATH         |    VISIBLE?        |    ENABLED?    #
      #----------------------------------------------------------------------------------------------#
    }
end

Tabulous will give me an error telling me that the config.tabs table is improperly formatted. So is there a way around this issue currently?

Thanks.

techiferous commented 11 years ago

Oh, I hadn't thought about that use case. Thanks for bringing it up. I think a quick fix to keep you going is to configure devise to use a GET instead of a DELETE for signing out; I think it has an option like this:

config.sign_out_via = :get

I know that is less than ideal, but it should keep you moving. I'll leave this issue open so that sometime in the future I can fix this.

samzhao commented 11 years ago

Thanks for your response. I was following the tutorial on Railsapp, and the reason why we should use a DELETE instead of GET is because

By default, Devise uses an http DELETE request for sign out requests
(destroy_user_session_path). Rails uses Javascript to implement
httpDELETE requests. Prior to Devise 1.4.1 (27 June 2011), Devise used an
http GET request for sign out requests. Jose Valim explained the change:
“GET requests should not change the state of the server. When sign out is
a GET request, CSRF can be used to sign you out automatically and things
that preload links can eventually sign you out by mistake as well.”

Therefore, I think I'll stick with DELETE. For now I'll just work on other parts of my application and I can deal with the authentication part later (since obviously it's not the core feature).

I think it'd be good to just include an extra parameter to the tabs, that way it's not tied to specific gems like Devise, and we have way more options than the defaults.

P.S. I'm not sure if github processes emails and apply markdown. On Nov 14, 2012 7:45 AM, "Wyatt Greene" notifications@github.com wrote:

Oh, I hadn't thought about that use case. Thanks for bringing it up. I think a quick fix to keep you going is to configure devise to use a GET instead of a DELETE for signing out; I think it has an option like this:

config.sign_out_via = :get

I know that is less than ideal, but it should keep you moving. I'll leave this issue open so that sometime in the future I can fix this.

— Reply to this email directly or view it on GitHubhttps://github.com/techiferous/tabulous/issues/20#issuecomment-10370813.

techiferous commented 11 years ago

Yes, this is exactly the reason why using GET is not ideal. Sorry tabulous is not helping you here; I will definitely keep this in mind.

samzhao commented 11 years ago

So is there a way to maybe override some default tabulous actions to make this happen?

techiferous commented 11 years ago

At the moment, I would recommend forking tabulous and tweaking tabulous yourself. Or write the tab code by hand--it's not too hard, just tedious.

samzhao commented 11 years ago

OK, I give up. The source is too hard for me to understand. I need to keep on learning. I'll just hard code the sign out link and render it after the tabulous tabs before tabulous can include a method parameter.

Thanks.

techiferous commented 11 years ago

Yes, the source code is admittedly a bit complex and hard to dive into; my apologies.

Thanks for your patience.

samzhao commented 11 years ago

I've been learning Rails for only a couple of months, so I shouldn't really blame you. Nonetheless, I will try to fork it later (when I've learned enough to be able to), if it still doesn't support Devise logout by then.

GreySyntax commented 11 years ago

I'm looking into adding this functionality, would there be a preferred syntax for specifying the method in the table? So far i'm considering something like:


[ :signup_tab, 'Sign Out', "#{destroy_user_session_path}:method=delete", user_signed_in? , true ],
samzhao commented 11 years ago

I'd say add method as an optional argument to the end:

signout_tab: { text: 'Sign out', path: destroy_user_session_path, visible: user_signed_in?, enabled: true, method: delete }
GreySyntax commented 11 years ago

Good point, i shall see what is required to make it work.

GreySyntax commented 11 years ago

Added support via a pull request the initial templates needs updating to say the 'method' is optional https://github.com/techiferous/tabulous/pull/23

samzhao commented 11 years ago

~(≧▽≦)/(≧▽≦)/(≧▽≦)/(≧▽≦)/(≧▽≦)/~