From d67055cce9a9f437a875a0fa51733a56b45ae006 Mon Sep 17 00:00:00 2001
From: dwindown <dwinx.ramz@gmail.com>
Date: Sun, 9 Nov 2025 22:22:36 +0700
Subject: [PATCH] fix: Modal refresh + improved accordion UX
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes:
✅ Modal now shows newly added methods immediately
✅ Accordion chevron on right (standard pattern)
✅ Remove button moved to content area

Changes:
1. Added useEffect to sync selectedZone with zones data
   - Modal now updates when methods are added/deleted

2. Restructured accordion:
   Before: [Truck Icon] Name/Price [Chevron] [Delete]
   After:  [Truck Icon] Name/Price [Chevron →]

3. Button layout in expanded content:
   [Remove] | [Cancel] [Save]

Benefits:
✅ Clearer visual hierarchy
✅ Remove action grouped with other actions
✅ Standard accordion pattern (chevron on right)
✅ Better mobile UX (no accidental deletes)

Next: Research shipping addon integration patterns
---
 admin-spa/src/routes/Settings/Shipping.tsx | 227 +++++++++++----------
 1 file changed, 119 insertions(+), 108 deletions(-)

diff --git a/admin-spa/src/routes/Settings/Shipping.tsx b/admin-spa/src/routes/Settings/Shipping.tsx
index 9cd7b3b..e008bda 100644
--- a/admin-spa/src/routes/Settings/Shipping.tsx
+++ b/admin-spa/src/routes/Settings/Shipping.tsx
@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React, { useState, useEffect } from 'react';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
 import { api } from '@/lib/api';
 import { SettingsLayout } from './components/SettingsLayout';
@@ -53,6 +53,16 @@ export default function ShippingPage() {
     enabled: showAddMethod,
   });
 
+  // Sync selectedZone with zones data when it changes
+  useEffect(() => {
+    if (selectedZone && zones.length > 0) {
+      const updatedZone = zones.find((z: any) => z.id === selectedZone.id);
+      if (updatedZone) {
+        setSelectedZone(updatedZone);
+      }
+    }
+  }, [zones]);
+
   // Toggle shipping method mutation
   const toggleMutation = useMutation({
     mutationFn: async ({ zoneId, instanceId, enabled }: { zoneId: number; instanceId: number; enabled: boolean }) => {
@@ -334,37 +344,24 @@ export default function ShippingPage() {
                   }}>
                     {selectedZone.rates?.map((rate: any) => (
                       <AccordionItem key={rate.id} value={String(rate.instance_id)} className="border rounded-lg mb-2">
-                        <div className="flex items-center justify-between pr-4">
-                          <AccordionTrigger className="flex-1 hover:no-underline px-4">
-                            <div className="flex items-center gap-3 flex-1">
-                              <Truck className="h-4 w-4 text-muted-foreground" />
-                              <div className="flex-1 text-left">
-                                <div className="font-medium" dangerouslySetInnerHTML={{ __html: rate.name }} />
-                                <div className="flex items-center gap-2 text-sm text-muted-foreground mt-1">
-                                  <span className="font-semibold" dangerouslySetInnerHTML={{ __html: rate.price }} />
-                                  <span className={`text-xs px-2 py-0.5 rounded-full ${
-                                    rate.enabled 
-                                      ? 'bg-green-100 text-green-700' 
-                                      : 'bg-gray-100 text-gray-600'
-                                  }`}>
-                                    {rate.enabled ? __('On') : __('Off')}
-                                  </span>
-                                </div>
+                        <AccordionTrigger className="hover:no-underline px-4">
+                          <div className="flex items-center gap-3 flex-1">
+                            <Truck className="h-4 w-4 text-muted-foreground" />
+                            <div className="flex-1 text-left">
+                              <div className="font-medium" dangerouslySetInnerHTML={{ __html: rate.name }} />
+                              <div className="flex items-center gap-2 text-sm text-muted-foreground mt-1">
+                                <span className="font-semibold" dangerouslySetInnerHTML={{ __html: rate.price }} />
+                                <span className={`text-xs px-2 py-0.5 rounded-full ${
+                                  rate.enabled 
+                                    ? 'bg-green-100 text-green-700' 
+                                    : 'bg-gray-100 text-gray-600'
+                                }`}>
+                                  {rate.enabled ? __('On') : __('Off')}
+                                </span>
                               </div>
                             </div>
-                          </AccordionTrigger>
-                          <Button
-                            variant="ghost"
-                            size="sm"
-                            onClick={(e) => {
-                              e.stopPropagation();
-                              handleDeleteMethod(rate.instance_id);
-                            }}
-                            disabled={deleteMethodMutation.isPending}
-                          >
-                            <Trash2 className="h-4 w-4 text-destructive" />
-                          </Button>
-                        </div>
+                          </div>
+                        </AccordionTrigger>
                         <AccordionContent className="px-4 pb-4">
                           {methodSettings[rate.instance_id] ? (
                             <div className="space-y-4 pt-2">
@@ -422,36 +419,50 @@ export default function ShippingPage() {
                                 </div>
                               )}
 
-                              <div className="flex justify-end gap-2 pt-2">
-                                <Button
-                                  variant="outline"
-                                  size="sm"
-                                  onClick={() => setExpandedMethod('')}
-                                >
-                                  {__('Cancel')}
-                                </Button>
+                              <div className="flex justify-between gap-2 pt-2">
                                 <Button
+                                  variant="destructive"
                                   size="sm"
                                   onClick={() => {
-                                    const settings: any = {};
-                                    const titleInput = document.getElementById(`method-title-${rate.instance_id}`) as HTMLInputElement;
-                                    const costInput = document.getElementById(`method-cost-${rate.instance_id}`) as HTMLInputElement;
-                                    const minAmountInput = document.getElementById(`method-min-amount-${rate.instance_id}`) as HTMLInputElement;
-                                    
-                                    if (titleInput) settings.title = titleInput.value;
-                                    if (costInput) settings.cost = costInput.value;
-                                    if (minAmountInput) settings.min_amount = minAmountInput.value;
-                                    
-                                    updateSettingsMutation.mutate({
-                                      zoneId: selectedZone.id,
-                                      instanceId: rate.instance_id,
-                                      settings
-                                    });
+                                    handleDeleteMethod(rate.instance_id);
+                                    setExpandedMethod('');
                                   }}
-                                  disabled={updateSettingsMutation.isPending}
+                                  disabled={deleteMethodMutation.isPending}
                                 >
-                                  {updateSettingsMutation.isPending ? __('Saving...') : __('Save')}
+                                  <Trash2 className="h-4 w-4 mr-1" />
+                                  {__('Remove')}
                                 </Button>
+                                <div className="flex gap-2">
+                                  <Button
+                                    variant="outline"
+                                    size="sm"
+                                    onClick={() => setExpandedMethod('')}
+                                  >
+                                    {__('Cancel')}
+                                  </Button>
+                                  <Button
+                                    size="sm"
+                                    onClick={() => {
+                                      const settings: any = {};
+                                      const titleInput = document.getElementById(`method-title-${rate.instance_id}`) as HTMLInputElement;
+                                      const costInput = document.getElementById(`method-cost-${rate.instance_id}`) as HTMLInputElement;
+                                      const minAmountInput = document.getElementById(`method-min-amount-${rate.instance_id}`) as HTMLInputElement;
+                                      
+                                      if (titleInput) settings.title = titleInput.value;
+                                      if (costInput) settings.cost = costInput.value;
+                                      if (minAmountInput) settings.min_amount = minAmountInput.value;
+                                      
+                                      updateSettingsMutation.mutate({
+                                        zoneId: selectedZone.id,
+                                        instanceId: rate.instance_id,
+                                        settings
+                                      });
+                                    }}
+                                    disabled={updateSettingsMutation.isPending}
+                                  >
+                                    {updateSettingsMutation.isPending ? __('Saving...') : __('Save')}
+                                  </Button>
+                                </div>
                               </div>
                             </div>
                           ) : (
@@ -512,38 +523,24 @@ export default function ShippingPage() {
                   }}>
                     {selectedZone.rates?.map((rate: any) => (
                       <AccordionItem key={rate.id} value={String(rate.instance_id)} className="border rounded-lg mb-2">
-                        <div className="flex items-center justify-between pr-2">
-                          <AccordionTrigger className="flex-1 hover:no-underline px-3 py-2">
-                            <div className="flex items-center gap-2 flex-1 min-w-0">
-                              <Truck className="h-3.5 w-3.5 text-muted-foreground flex-shrink-0" />
-                              <div className="flex-1 text-left min-w-0">
-                                <div className="font-medium text-sm line-clamp-1" dangerouslySetInnerHTML={{ __html: rate.name }} />
-                                <div className="flex items-center gap-2 text-xs text-muted-foreground mt-0.5">
-                                  <span className="font-semibold" dangerouslySetInnerHTML={{ __html: rate.price }} />
-                                  <span className={`px-1.5 py-0.5 rounded-full whitespace-nowrap ${
-                                    rate.enabled 
-                                      ? 'bg-green-100 text-green-700' 
-                                      : 'bg-gray-100 text-gray-600'
-                                  }`}>
-                                    {rate.enabled ? __('On') : __('Off')}
-                                  </span>
-                                </div>
+                        <AccordionTrigger className="hover:no-underline px-3 py-2">
+                          <div className="flex items-center gap-2 flex-1 min-w-0">
+                            <Truck className="h-3.5 w-3.5 text-muted-foreground flex-shrink-0" />
+                            <div className="flex-1 text-left min-w-0">
+                              <div className="font-medium text-sm line-clamp-1" dangerouslySetInnerHTML={{ __html: rate.name }} />
+                              <div className="flex items-center gap-2 text-xs text-muted-foreground mt-0.5">
+                                <span className="font-semibold" dangerouslySetInnerHTML={{ __html: rate.price }} />
+                                <span className={`px-1.5 py-0.5 rounded-full whitespace-nowrap ${
+                                  rate.enabled 
+                                    ? 'bg-green-100 text-green-700' 
+                                    : 'bg-gray-100 text-gray-600'
+                                }`}>
+                                  {rate.enabled ? __('On') : __('Off')}
+                                </span>
                               </div>
                             </div>
-                          </AccordionTrigger>
-                          <Button
-                            variant="ghost"
-                            size="sm"
-                            className="h-7 w-7 p-0"
-                            onClick={(e) => {
-                              e.stopPropagation();
-                              handleDeleteMethod(rate.instance_id);
-                            }}
-                            disabled={deleteMethodMutation.isPending}
-                          >
-                            <Trash2 className="h-3 w-3 text-destructive" />
-                          </Button>
-                        </div>
+                          </div>
+                        </AccordionTrigger>
                         <AccordionContent className="px-3 pb-3">
                           {methodSettings[rate.instance_id] ? (
                             <div className="space-y-3 pt-2">
@@ -592,36 +589,50 @@ export default function ShippingPage() {
                                 </div>
                               )}
 
-                              <div className="flex justify-end gap-2 pt-1">
-                                <Button
-                                  variant="outline"
-                                  size="sm"
-                                  onClick={() => setExpandedMethod('')}
-                                >
-                                  {__('Cancel')}
-                                </Button>
+                              <div className="flex justify-between gap-2 pt-1">
                                 <Button
+                                  variant="destructive"
                                   size="sm"
                                   onClick={() => {
-                                    const settings: any = {};
-                                    const titleInput = document.getElementById(`method-title-mobile-${rate.instance_id}`) as HTMLInputElement;
-                                    const costInput = document.getElementById(`method-cost-mobile-${rate.instance_id}`) as HTMLInputElement;
-                                    const minAmountInput = document.getElementById(`method-min-amount-mobile-${rate.instance_id}`) as HTMLInputElement;
-                                    
-                                    if (titleInput) settings.title = titleInput.value;
-                                    if (costInput) settings.cost = costInput.value;
-                                    if (minAmountInput) settings.min_amount = minAmountInput.value;
-                                    
-                                    updateSettingsMutation.mutate({
-                                      zoneId: selectedZone.id,
-                                      instanceId: rate.instance_id,
-                                      settings
-                                    });
+                                    handleDeleteMethod(rate.instance_id);
+                                    setExpandedMethod('');
                                   }}
-                                  disabled={updateSettingsMutation.isPending}
+                                  disabled={deleteMethodMutation.isPending}
                                 >
-                                  {updateSettingsMutation.isPending ? __('Saving...') : __('Save')}
+                                  <Trash2 className="h-3 w-3 mr-1" />
+                                  {__('Remove')}
                                 </Button>
+                                <div className="flex gap-2">
+                                  <Button
+                                    variant="outline"
+                                    size="sm"
+                                    onClick={() => setExpandedMethod('')}
+                                  >
+                                    {__('Cancel')}
+                                  </Button>
+                                  <Button
+                                    size="sm"
+                                    onClick={() => {
+                                      const settings: any = {};
+                                      const titleInput = document.getElementById(`method-title-mobile-${rate.instance_id}`) as HTMLInputElement;
+                                      const costInput = document.getElementById(`method-cost-mobile-${rate.instance_id}`) as HTMLInputElement;
+                                      const minAmountInput = document.getElementById(`method-min-amount-mobile-${rate.instance_id}`) as HTMLInputElement;
+                                      
+                                      if (titleInput) settings.title = titleInput.value;
+                                      if (costInput) settings.cost = costInput.value;
+                                      if (minAmountInput) settings.min_amount = minAmountInput.value;
+                                      
+                                      updateSettingsMutation.mutate({
+                                        zoneId: selectedZone.id,
+                                        instanceId: rate.instance_id,
+                                        settings
+                                      });
+                                    }}
+                                    disabled={updateSettingsMutation.isPending}
+                                  >
+                                    {updateSettingsMutation.isPending ? __('Saving...') : __('Save')}
+                                  </Button>
+                                </div>
                               </div>
                             </div>
                           ) : (