Skip to content

Commit 3d8ebaf

Browse files
Copilothotlong
andcommitted
fix: address code review feedback — null check, typed actionType, validation errors, input types
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 2531d91 commit 3d8ebaf

3 files changed

Lines changed: 44 additions & 6 deletions

File tree

ROADMAP.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,6 +1398,31 @@ All 313 `@object-ui/fields` tests pass.
13981398

13991399
---
14001400

1401+
### RecordDetailView — Action Button Full-Chain Integration (March 2026)
1402+
1403+
> **Issue #107:** All Action buttons on record detail pages (Change Stage, Mark as Won, etc.) clicked with zero response — no dialogs, no API calls, no toast, no data refresh.
1404+
1405+
**Root Causes (6 independent bugs):**
1406+
1407+
1. **Missing `ActionProvider`**`RecordDetailView` didn't wrap `DetailView` with `ActionProvider`, so `useAction()` fell back to an empty `ActionRunner` with no handlers.
1408+
2. **Action type overwritten**`action:bar` component overrode `action.type` (`'api'`) with the component type (`'action:button'`), so `ActionRunner` never matched the registered `'api'` handler.
1409+
3. **No API handler**`api` action targets were logical names (e.g., `'opportunity_change_stage'`), not HTTP URLs. The built-in `executeAPI()` tried `fetch('opportunity_change_stage')` which failed silently.
1410+
4. **No param collection**`ActionParam[]` was passed as `params` (values) instead of `actionParams` (definitions to collect), so the param collection dialog was never triggered.
1411+
5. **No confirm/toast handlers**`confirmText` fell back to `window.confirm`, success/error messages were silently dropped.
1412+
6. **No visibility context**`useCondition` evaluated `visible` expressions like `"stage !== 'closed_won'"` with empty context, always returning `true`.
1413+
1414+
**Fix:**
1415+
1416+
- **RecordDetailView** (`apps/console/src/components/RecordDetailView.tsx`): Wrapped `DetailView` with `<ActionProvider>` providing `onConfirm`, `onToast`, `onNavigate`, `onParamCollection` handlers and a custom `api` handler that maps logical action targets to `dataSource.update()` operations.
1417+
- **action-bar** (`packages/components/src/renderers/action/action-bar.tsx`): Preserves original `action.type` as `actionType` when overriding with component type. Forwards `data` prop to child action renderers for visibility context.
1418+
- **action-button** (`packages/components/src/renderers/action/action-button.tsx`): Uses `actionType` for execution. Detects `ActionParam[]` arrays and passes as `actionParams`. Passes record `data` to `useCondition` for visibility expressions.
1419+
- **ActionConfirmDialog** (`apps/console/src/components/ActionConfirmDialog.tsx`): Promise-based confirmation dialog using Shadcn `AlertDialog`.
1420+
- **ActionParamDialog** (`apps/console/src/components/ActionParamDialog.tsx`): Dynamic form dialog for collecting action parameters (select, text, textarea) using Shadcn `Dialog`.
1421+
1422+
**Tests:** 6 new integration tests covering: action button rendering, confirm dialog show/accept/cancel, param collection dialog, toast notification, dataSource.update invocation. All 764 console tests pass.
1423+
1424+
---
1425+
14011426
## ⚠️ Risk Management
14021427

14031428
| Risk | Mitigation |

apps/console/src/components/ActionParamDialog.tsx

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ interface ActionParamDialogProps {
4343

4444
export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProps) {
4545
const [values, setValues] = useState<Record<string, any>>({});
46+
const [errors, setErrors] = useState<Record<string, boolean>>({});
4647

4748
// Reset values when params change
4849
useEffect(() => {
@@ -54,16 +55,22 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
5455
}
5556
}
5657
setValues(defaults);
58+
setErrors({});
5759
}
5860
}, [state.open, state.params]);
5961

6062
const handleSubmit = () => {
6163
// Validate required fields
64+
const newErrors: Record<string, boolean> = {};
6265
for (const param of state.params) {
6366
if (param.required && !values[param.name]) {
64-
return; // Don't submit if required fields are empty
67+
newErrors[param.name] = true;
6568
}
6669
}
70+
if (Object.keys(newErrors).length > 0) {
71+
setErrors(newErrors);
72+
return;
73+
}
6774
state.resolve?.(values);
6875
onOpenChange(false);
6976
};
@@ -75,6 +82,7 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
7582

7683
const updateValue = (name: string, value: any) => {
7784
setValues(prev => ({ ...prev, [name]: value }));
85+
setErrors(prev => ({ ...prev, [name]: false }));
7886
};
7987

8088
return (
@@ -102,7 +110,7 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
102110
value={values[param.name] || ''}
103111
onValueChange={(val) => updateValue(param.name, val)}
104112
>
105-
<SelectTrigger id={param.name}>
113+
<SelectTrigger id={param.name} className={errors[param.name] ? 'border-destructive' : ''}>
106114
<SelectValue placeholder={param.placeholder || `Select ${param.label}`} />
107115
</SelectTrigger>
108116
<SelectContent>
@@ -119,17 +127,22 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
119127
value={values[param.name] || ''}
120128
onChange={(e) => updateValue(param.name, e.target.value)}
121129
placeholder={param.placeholder}
130+
className={errors[param.name] ? 'border-destructive' : ''}
122131
/>
123132
) : (
124133
<Input
125134
id={param.name}
126-
type={param.type === 'number' ? 'number' : 'text'}
135+
type={(['number', 'email', 'url', 'date', 'datetime-local', 'time', 'password'] as string[]).includes(param.type) ? param.type : 'text'}
127136
value={values[param.name] || ''}
128137
onChange={(e) => updateValue(param.name, e.target.value)}
129138
placeholder={param.placeholder}
139+
className={errors[param.name] ? 'border-destructive' : ''}
130140
/>
131141
)}
132142

143+
{errors[param.name] && (
144+
<p className="text-xs text-destructive">{param.label} is required</p>
145+
)}
133146
{param.helpText && (
134147
<p className="text-xs text-muted-foreground">{param.helpText}</p>
135148
)}

packages/components/src/renderers/action/action-button.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { Loader2 } from 'lucide-react';
2828
import { resolveIcon } from './resolve-icon';
2929

3030
export interface ActionButtonProps {
31-
schema: ActionSchema & { type: string; className?: string };
31+
schema: ActionSchema & { type: string; className?: string; actionType?: string };
3232
className?: string;
3333
/** Override context for this specific action */
3434
context?: Record<string, any>;
@@ -48,7 +48,7 @@ const ActionButtonRenderer = forwardRef<HTMLButtonElement, ActionButtonProps>(
4848
const [loading, setLoading] = useState(false);
4949

5050
// Record data may be passed from SchemaRenderer (e.g. DetailView passes record data)
51-
const recordData = rest.data && typeof rest.data === 'object' ? rest.data as Record<string, any> : {};
51+
const recordData = rest.data != null && typeof rest.data === 'object' ? rest.data as Record<string, any> : {};
5252

5353
// Evaluate visibility and enabled conditions with record data context
5454
const isVisible = useCondition(schema.visible ? `\${${schema.visible}}` : undefined, recordData);
@@ -72,7 +72,7 @@ const ActionButtonRenderer = forwardRef<HTMLButtonElement, ActionButtonProps>(
7272
typeof schema.params[0] === 'object' && 'name' in schema.params[0] && 'type' in schema.params[0];
7373

7474
await execute({
75-
type: (schema as any).actionType || schema.type,
75+
type: schema.actionType || schema.type,
7676
name: schema.name,
7777
target: schema.target,
7878
execute: schema.execute,

0 commit comments

Comments
 (0)