From 0c538de698d4454c2c105a46c8c0fbf3b08dd0be Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Nov 2025 13:01:44 +0000 Subject: [PATCH] Refactor: Improve trip sharing UX with Turbo and better controls Major improvements: 1. Use Turbo for sharing updates - no page reload, modal stays open 2. Add Stimulus copy button controller - clean implementation with 'Copied!' feedback 3. Allow updating notes/photos toggles without disabling sharing 4. Add 'Update Sharing' button to save changes while keeping sharing enabled 5. Use 'true'/'false' strings consistently instead of '1'/'0' 6. Update all request specs to use 'true'/'false' values Technical changes: - Wrap form in turbo_frame_tag for seamless updates - Controller responds with turbo_stream to replace form content - Create copy_button_controller.js for proper copy feedback - Checkboxes now editable when sharing is enabled - Separate 'Update Sharing' and 'Disable Sharing' actions --- app/controllers/trips_controller.rb | 7 +- .../controllers/copy_button_controller.js | 31 ++ app/views/trips/_sharing.html.erb | 383 +++++++++--------- spec/requests/shared/trips_spec.rb | 18 +- 4 files changed, 239 insertions(+), 200 deletions(-) create mode 100644 app/javascript/controllers/copy_button_controller.js diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index d4921f01..844ac761 100644 --- a/app/controllers/trips_controller.rb +++ b/app/controllers/trips_controller.rb @@ -42,7 +42,12 @@ class TripsController < ApplicationController # Handle sharing settings update if params[:trip] && params[:trip][:sharing] handle_sharing_update - redirect_to @trip, notice: 'Trip was successfully updated.', status: :see_other and return + + respond_to do |format| + format.turbo_stream { render turbo_stream: turbo_stream.replace("sharing_form_#{@trip.id}", partial: 'trips/sharing', locals: { trip: @trip }) } + format.html { redirect_to @trip, notice: 'Trip was successfully updated.', status: :see_other } + end + return end # Handle regular trip update diff --git a/app/javascript/controllers/copy_button_controller.js b/app/javascript/controllers/copy_button_controller.js new file mode 100644 index 00000000..13fdb15b --- /dev/null +++ b/app/javascript/controllers/copy_button_controller.js @@ -0,0 +1,31 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = ["button"] + static values = { text: String } + + copy(event) { + event.preventDefault() + const text = event.currentTarget.dataset.copyText + + navigator.clipboard.writeText(text).then(() => { + const button = event.currentTarget + const originalHTML = button.innerHTML + + // Show "Copied!" feedback + button.innerHTML = ` + + + + Copied! + ` + + // Restore original content after 2 seconds + setTimeout(() => { + button.innerHTML = originalHTML + }, 2000) + }).catch(err => { + console.error('Failed to copy text: ', err) + }) + } +} diff --git a/app/views/trips/_sharing.html.erb b/app/views/trips/_sharing.html.erb index 51f1952e..3027ebdc 100644 --- a/app/views/trips/_sharing.html.erb +++ b/app/views/trips/_sharing.html.erb @@ -11,200 +11,203 @@ Trip Sharing Settings - <%= form_with model: trip, url: trip_path(trip), method: :patch, data: { turbo: false } do |f| %> - <% if trip.sharing_enabled? %> - -
- -
- <% if trip.sharing_expired? %> - -
-

Link Expired

-

This share link has expired and is no longer accessible

-
- <% else %> - -
-

Sharing Active

-

This trip is publicly accessible via the link below

-
- <% end %> -
+ <%= turbo_frame_tag "sharing_form_#{trip.id}" do %> + <%= form_with model: trip, url: trip_path(trip), method: :patch, id: "sharing_form_#{trip.id}" do |f| %> + <% if trip.sharing_enabled? %> + +
+ +
+ <% if trip.sharing_expired? %> + +
+

Link Expired

+

This share link has expired and is no longer accessible

+
+ <% else %> + +
+

Sharing Active

+

This trip is publicly accessible via the link below

+
+ <% end %> +
- -
- -
- - +
+
+ Share this link with others to give them access to your trip +
+
+ + +
+ <%= link_to shared_trip_path(trip.sharing_uuid), target: '_blank', class: 'btn btn-outline btn-primary gap-2' do %> + + + + View Public Page + <% end %> +
+ + +
+
+

What to share:

+ +
+
+ +
+
+ +
+
+ +
+ Always included: Trip name, dates, route map, distance, and countries visited +
+
+
+ + +
+ +
+

Privacy Protection

+
+ • Only selected information is shared
+ • Personal information and account details are never included
+ • You can disable sharing at any time +
+
+
+ + + <%= hidden_field_tag 'trip[sharing][enabled]', 'true' %> +
+ <%= submit_tag "Update Sharing", + class: "btn btn-primary" %> + <%= submit_tag "Disable Sharing", + name: 'trip[sharing][enabled]', + value: 'false', + class: "btn btn-error", + data: { turbo_confirm: "Are you sure you want to disable sharing for this trip?" } %> +
+
+ <% else %> + +
+

+ Generate a public link to share this trip with friends, family, or on social media. +

+ + +
+
+

Choose what to share:

+ +
+ + <%= select_tag 'trip[sharing][expiration]', + options_for_select([ + ['1 hour', '1h'], + ['12 hours', '12h'], + ['24 hours', '24h'], + ['Never (permanent)', 'permanent'] + ], '24h'), + class: 'select select-bordered w-full' %> +
+ Choose how long the sharing link will remain active +
+
+ +
+ +
+
+ +
+
+ +
+
+ +
+ Always included: Trip name, dates, route map, distance, and countries visited +
+
+
+ + +
+ +
+

Privacy Protection

+
+ • Only selected information will be shared
+ • Personal information and account details are never included
+ • You can disable sharing at any time +
+
+
+ + + <%= hidden_field_tag 'trip[sharing][enabled]', 'true' %> +
+ -
-
- Share this link with others to give them access to your trip + <%= submit_tag "Enable Sharing", class: "btn btn-primary" %>
- - -
- <%= link_to shared_trip_path(trip.sharing_uuid), target: '_blank', class: 'btn btn-outline btn-primary gap-2' do %> - - - - View Public Page - <% end %> -
- - -
-
-

What's being shared:

-
-
- <% if trip.share_notes? %> - - <% else %> - - <% end %> - Trip notes -
-
- <% if trip.share_photos? %> - - <% else %> - - <% end %> - Photos -
-
-
- Always included: Trip name, dates, route map, distance, and countries visited -
-
-
- - -
- -
-

Privacy Protection

-
- • Only selected information is shared
- • Personal information and account details are never included
- • You can disable sharing at any time -
-
-
- - -
- <%= submit_tag "Disable Sharing", - name: 'trip[sharing][enabled]', - value: '0', - class: "btn btn-error", - data: { turbo_confirm: "Are you sure you want to disable sharing for this trip?" } %> - -
-
- <% else %> - -
-

- Generate a public link to share this trip with friends, family, or on social media. -

- - -
-
-

Choose what to share:

- -
- - <%= select_tag 'trip[sharing][expiration]', - options_for_select([ - ['1 hour', '1h'], - ['12 hours', '12h'], - ['24 hours', '24h'], - ['Never (permanent)', 'permanent'] - ], '24h'), - class: 'select select-bordered w-full' %> -
- Choose how long the sharing link will remain active -
-
- -
- -
-
- -
-
- -
-
- -
- Always included: Trip name, dates, route map, distance, and countries visited -
-
-
- - -
- -
-

Privacy Protection

-
- • Only selected information will be shared
- • Personal information and account details are never included
- • You can disable sharing at any time -
-
-
- - - <%= hidden_field_tag 'trip[sharing][enabled]', '1' %> -
- - <%= submit_tag "Enable Sharing", class: "btn btn-primary" %> -
-
+ <% end %> <% end %> <% end %> diff --git a/spec/requests/shared/trips_spec.rb b/spec/requests/shared/trips_spec.rb index cf5eed8f..11eb2ed7 100644 --- a/spec/requests/shared/trips_spec.rb +++ b/spec/requests/shared/trips_spec.rb @@ -119,7 +119,7 @@ RSpec.describe 'Shared::Trips', type: :request do context 'enabling sharing' do it 'enables sharing and redirects to trip' do patch trip_path(trip), - params: { trip: { sharing: { enabled: '1', expiration: '24h' } } } + params: { trip: { sharing: { enabled: 'true', expiration: '24h' } } } expect(response).to redirect_to(trip_path(trip)) expect(flash[:notice]).to eq('Trip was successfully updated.') @@ -131,7 +131,7 @@ RSpec.describe 'Shared::Trips', type: :request do it 'enables sharing with notes option' do patch trip_path(trip), - params: { trip: { sharing: { enabled: '1', expiration: '24h', share_notes: '1' } } } + params: { trip: { sharing: { enabled: 'true', expiration: '24h', share_notes: 'true' } } } expect(response).to redirect_to(trip_path(trip)) @@ -142,7 +142,7 @@ RSpec.describe 'Shared::Trips', type: :request do it 'enables sharing with photos option' do patch trip_path(trip), - params: { trip: { sharing: { enabled: '1', expiration: '24h', share_photos: '1' } } } + params: { trip: { sharing: { enabled: 'true', expiration: '24h', share_photos: 'true' } } } expect(response).to redirect_to(trip_path(trip)) @@ -153,7 +153,7 @@ RSpec.describe 'Shared::Trips', type: :request do it 'sets custom expiration when provided' do patch trip_path(trip), - params: { trip: { sharing: { enabled: '1', expiration: '1h' } } } + params: { trip: { sharing: { enabled: 'true', expiration: '1h' } } } expect(response).to redirect_to(trip_path(trip)) trip.reload @@ -163,7 +163,7 @@ RSpec.describe 'Shared::Trips', type: :request do it 'enables permanent sharing' do patch trip_path(trip), - params: { trip: { sharing: { enabled: '1', expiration: 'permanent' } } } + params: { trip: { sharing: { enabled: 'true', expiration: 'permanent' } } } expect(response).to redirect_to(trip_path(trip)) trip.reload @@ -179,7 +179,7 @@ RSpec.describe 'Shared::Trips', type: :request do it 'disables sharing and redirects to trip' do patch trip_path(trip), - params: { trip: { sharing: { enabled: '0' } } } + params: { trip: { sharing: { enabled: 'false' } } } expect(response).to redirect_to(trip_path(trip)) expect(flash[:notice]).to eq('Trip was successfully updated.') @@ -192,7 +192,7 @@ RSpec.describe 'Shared::Trips', type: :request do context 'when trip does not exist' do it 'returns not found' do patch trip_path(id: 999999), - params: { trip: { sharing: { enabled: '1' } } } + params: { trip: { sharing: { enabled: 'true' } } } expect(response).to have_http_status(:not_found) end @@ -204,7 +204,7 @@ RSpec.describe 'Shared::Trips', type: :request do it 'returns not found' do patch trip_path(other_trip), - params: { trip: { sharing: { enabled: '1' } } } + params: { trip: { sharing: { enabled: 'true' } } } expect(response).to have_http_status(:not_found) end @@ -214,7 +214,7 @@ RSpec.describe 'Shared::Trips', type: :request do context 'when user is not signed in' do it 'returns unauthorized' do patch trip_path(trip), - params: { trip: { sharing: { enabled: '1' } } } + params: { trip: { sharing: { enabled: 'true' } } } expect(response).to have_http_status(:unauthorized) end