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

Feature: Use delete request with sign out #3881

Merged
merged 1 commit into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/components/nav/item_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<% if mobile %>
<%= link_to path, class: "text-gray-600 hover:bg-gray-50 hover:text-gray-900 group flex items-center px-2 py-2 text-base font-medium dark:text-gray-300 dark:hover:text-gray-200 dark:hover:bg-gray-700/60 #{'bg-gray-100 text-gray-900 dark:bg-gray-700/60 dark:text-gray-3 00' if active?}", data: { test_id: } do %>
<% if mobile? %>
<%= link_to path, class: "text-gray-600 hover:bg-gray-50 hover:text-gray-900 group flex items-center px-2 py-2 text-base font-medium dark:text-gray-300 dark:hover:text-gray-200 dark:hover:bg-gray-700/60 #{'bg-gray-100 text-gray-900 dark:bg-gray-700/60 dark:text-gray-3 00' if active?}", data: { test_id:, turbo_method: http_method } do %>
<%= inline_svg_tag icon_path, class: 'text-gray-400 group-hover:text-gray-500 dark:text-gray-200 dark:group-hover:text-gray-200 mr-4 h-6 w-6', aria: true, title: "#{text} icon" %>
<%= text %>
<% end %>
<% else %>
<%= link_to text, path, class: " text-sm border-transparent text-gray-500 dark:text-gray-300 hover:border-gray-300 hover:text-gray-700 inline-flex items-center px-1 pt-1 border-b-2 font-medium #{'border-gold-500 text-gray-900' if active?}", data: { test_id: } %>
<%= link_to text, path, class: " text-sm border-transparent text-gray-500 dark:text-gray-300 hover:border-gray-300 hover:text-gray-700 inline-flex items-center px-1 pt-1 border-b-2 font-medium #{'border-gold-500 text-gray-900' if active?}", data: { test_id:, turbo_method: http_method } %>
<% end %>
14 changes: 11 additions & 3 deletions app/components/nav/item_component.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
class Nav::ItemComponent < ApplicationComponent
def initialize(path:, text:, test_id:, icon_path: nil, mobile: false)
def initialize(path:, text:, test_id:, icon_path: nil, options: {})
@path = path
@text = text
@test_id = test_id
@icon_path = icon_path
@mobile = mobile
@options = options
end

def active?
Expand All @@ -13,5 +13,13 @@ def active?

private

attr_reader :path, :text, :test_id, :icon_path, :mobile
attr_reader :path, :text, :test_id, :icon_path, :options

def http_method
options.fetch(:method, :get)
end

def mobile?
options.fetch(:mobile, false)
end
end
2 changes: 1 addition & 1 deletion app/components/user/profile_dropdown_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<%= render Theme::SwitcherComponent.new(current_theme:) %>
<% end %>

<%= link_to sign_out_path, class: 'text-gray-700 dark:text-gray-300 group flex items-center px-3 py-2 text-sm' do %>
<%= button_to sign_out_path, class: 'text-gray-700 dark:text-gray-300 group flex items-center px-3 py-2 text-sm', method: :delete do %>
<%= inline_svg_tag 'icons/sign-out.svg', class: 'mr-3 h-5 w-5 text-gray-400 group-hover:text-gray-500 dark:text-gray-300 dark:group-hover:text-gray-400', aria: true, title: 'Sign out icon' %>
Sign out
<% end %>
Expand Down
24 changes: 12 additions & 12 deletions app/views/shared/_off_canvas_menu.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -53,31 +53,31 @@
<nav aria-label="Sidebar" class="mt-3">
<div class="px-2 space-y-1">
<% if user_signed_in? %>
<%= render Nav::ItemComponent.new(path: dashboard_path, text: 'Dashboard', test_id: 'nav-dashboard', icon_path: 'icons/home.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: paths_url, text: 'All Paths', test_id: 'nav-all-paths', icon_path: 'icons/map.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: notifications_path, text: 'Notifications', test_id: 'nav-notifications', icon_path: 'icons/bell.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: ODIN_CHAT_URL, text: 'Community', test_id: 'nav-community', icon_path: 'icons/speech-bubbles.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: dashboard_path, text: 'Dashboard', test_id: 'nav-dashboard', icon_path: 'icons/home.svg', options: { mobile: true }) %>
<%= render Nav::ItemComponent.new(path: paths_url, text: 'All Paths', test_id: 'nav-all-paths', icon_path: 'icons/map.svg', options: { mobile: true }) %>
<%= render Nav::ItemComponent.new(path: notifications_path, text: 'Notifications', test_id: 'nav-notifications', icon_path: 'icons/bell.svg', options: { mobile: true }) %>
<%= render Nav::ItemComponent.new(path: ODIN_CHAT_URL, text: 'Community', test_id: 'nav-community', icon_path: 'icons/speech-bubbles.svg', options: { mobile: true }) %>
<% else %>
<%= render Nav::ItemComponent.new(path: root_path, text: 'Home', test_id: 'nav-home', icon_path: 'icons/home.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: paths_url, text: 'All Paths', test_id: 'nav-all-paths', icon_path: 'icons/map.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: about_path, text: 'About', test_id: 'nav-about', icon_path: 'icons/information.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: ODIN_CHAT_URL, text: 'Community', test_id: 'nav-community', icon_path: 'icons/speech-bubbles.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: root_path, text: 'Home', test_id: 'nav-home', icon_path: 'icons/home.svg', options: { mobile: true }) %>
<%= render Nav::ItemComponent.new(path: paths_url, text: 'All Paths', test_id: 'nav-all-paths', icon_path: 'icons/map.svg', options: { mobile: true }) %>
<%= render Nav::ItemComponent.new(path: about_path, text: 'About', test_id: 'nav-about', icon_path: 'icons/information.svg', options: { mobile: true }) %>
<%= render Nav::ItemComponent.new(path: ODIN_CHAT_URL, text: 'Community', test_id: 'nav-community', icon_path: 'icons/speech-bubbles.svg', options: { mobile: true }) %>
<% end %>
</div>
<hr class="border-t border-gray-200 my-4" aria-hidden="true">
<div class="px-2 space-y-1">
<% if user_signed_in? %>
<%= render Nav::ItemComponent.new(path: edit_users_profile_path, text: 'Settings', test_id: 'nav-settings', icon_path: 'icons/gear.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: edit_users_profile_path, text: 'Settings', test_id: 'nav-settings', icon_path: 'icons/gear.svg', options: { mobile: true }) %>
<%= turbo_frame_tag 'theme_switcher_mobile' do %>
<%= render Theme::SwitcherComponent.new(current_theme:, type: :mobile) %>
<% end %>
<%= render Nav::ItemComponent.new(path: sign_out_path, text: 'Sign out', test_id: 'nav-sign-out', icon_path: 'icons/sign-out.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: sign_out_path, text: 'Sign out', test_id: 'nav-sign-out', icon_path: 'icons/sign-out.svg', options: { method: :delete, mobile: true }) %>
<% else %>
<%= render Nav::ItemComponent.new(path: sign_up_path, text: 'Get started', test_id: 'nav-sign-up', icon_path: 'icons/rocket.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: sign_up_path, text: 'Get started', test_id: 'nav-sign-up', icon_path: 'icons/rocket.svg', options: { mobile: true }) %>
<%= turbo_frame_tag 'theme_switcher_mobile' do %>
<%= render Theme::SwitcherComponent.new(current_theme:, type: :mobile) %>
<% end %>
<%= render Nav::ItemComponent.new(path: sign_in_path, text: 'Sign in', test_id: 'nav-sign-in', icon_path: 'icons/sign-in.svg', mobile: true) %>
<%= render Nav::ItemComponent.new(path: sign_in_path, text: 'Sign in', test_id: 'nav-sign-in', icon_path: 'icons/sign-in.svg', options: { mobile: true }) %>
<% end %>
</div>
</nav>
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

devise_scope :user do
get '/sign_in' => 'users/sessions#new'
get '/sign_out' => 'users/sessions#destroy', method: :delete
delete '/sign_out' => 'users/sessions#destroy'
get '/sign_up' => 'users/registrations#new'
end

Expand Down
20 changes: 18 additions & 2 deletions spec/components/nav/item_component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
text: 'Home',
test_id: 'nav-home',
icon_path: 'icons/home.svg',
mobile: true
options: { mobile: true }
)

render_inline(component)
Expand All @@ -25,12 +25,28 @@
text: 'Home',
test_id: 'nav-home',
icon_path: nil,
mobile: false
)

render_inline(component)

expect(page).to have_link('Home', href: '/home')
end
end

context 'when nav item uses a different http request' do
it 'includes the different http request' do
component = described_class.new(
path: '/sign_out',
text: 'Sign out',
test_id: 'nav-sign-out',
icon_path: nil,
options: { method: :delete }
)

render_inline(component)

expect(page).to have_link('Sign out', href: '/sign_out')
expect(page.find_link('Sign out')['data-turbo-method']).to eq('delete')
end
end
end