Skip to content

Commit 692dffc

Browse files
Copilothotlong
andcommitted
fix: address code review feedback — dep array, style simplification, extract formatActionLabel
- Add normalizedQuickFilters to data fetch effect dependency array - Simplify conditionalFormatting style merge (remove redundant assignments) - Extract formatActionLabel helper in both ListView.tsx and ObjectGrid.tsx Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent cda2341 commit 692dffc

File tree

2 files changed

+21
-11
lines changed

2 files changed

+21
-11
lines changed

packages/plugin-grid/src/ObjectGrid.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,14 @@ export interface ObjectGridProps {
5050
onAddRecord?: () => void;
5151
}
5252

53+
/**
54+
* Format an action identifier string into a human-readable label.
55+
* e.g., 'send_email' → 'Send Email'
56+
*/
57+
function formatActionLabel(action: string): string {
58+
return action.replace(/_/g, ' ').replace(/\b\w/g, c => c.toUpperCase());
59+
}
60+
5361
/**
5462
* Helper to get data configuration from schema
5563
* Handles both new ViewData format and legacy inline data
@@ -762,7 +770,7 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
762770
onClick={() => executeAction({ type: action, params: { record: row } })}
763771
data-testid={`row-action-${action}`}
764772
>
765-
{action.replace(/_/g, ' ').replace(/\b\w/g, c => c.toUpperCase())}
773+
{formatActionLabel(action)}
766774
</DropdownMenuItem>
767775
))}
768776
</DropdownMenuContent>

packages/plugin-list/src/ListView.tsx

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,14 @@ export function normalizeFilterCondition(condition: any[]): any[] {
8585
return condition;
8686
}
8787

88+
/**
89+
* Format an action identifier string into a human-readable label.
90+
* e.g., 'send_email' → 'Send Email'
91+
*/
92+
function formatActionLabel(action: string): string {
93+
return action.replace(/_/g, ' ').replace(/\b\w/g, c => c.toUpperCase());
94+
}
95+
8896
/**
8997
* Normalize an array of filter conditions, expanding `in`/`not in` operators
9098
* and ensuring consistent AST structure.
@@ -167,15 +175,9 @@ export function evaluateConditionalFormatting(
167175
}
168176

169177
if (match) {
170-
// Merge: spec 'style' object takes precedence, then individual style properties
178+
// Build style: spec 'style' object is base, individual properties override
171179
const style: React.CSSProperties = {};
172-
if (rule.style) {
173-
if (rule.style.backgroundColor) style.backgroundColor = rule.style.backgroundColor;
174-
if (rule.style.color) style.color = rule.style.color;
175-
if (rule.style.borderColor) style.borderColor = rule.style.borderColor;
176-
// Spread any additional style properties from spec format
177-
Object.assign(style, rule.style);
178-
}
180+
if (rule.style) Object.assign(style, rule.style);
179181
if (rule.backgroundColor) style.backgroundColor = rule.backgroundColor;
180182
if (rule.textColor) style.color = rule.textColor;
181183
if (rule.borderColor) style.borderColor = rule.borderColor;
@@ -583,7 +585,7 @@ export const ListView: React.FC<ListViewProps> = ({
583585
fetchData();
584586

585587
return () => { isMounted = false; };
586-
}, [schema.objectName, schema.data, dataSource, schema.filters, effectivePageSize, currentSort, currentFilters, activeQuickFilters, userFilterConditions, refreshKey, searchTerm, schema.searchableFields]); // Re-fetch on filter/sort/search change
588+
}, [schema.objectName, schema.data, dataSource, schema.filters, effectivePageSize, currentSort, currentFilters, activeQuickFilters, normalizedQuickFilters, userFilterConditions, refreshKey, searchTerm, schema.searchableFields]); // Re-fetch on filter/sort/search change
587589

588590
// Available view types based on schema configuration
589591
const availableViews = React.useMemo(() => {
@@ -1468,7 +1470,7 @@ export const ListView: React.FC<ListViewProps> = ({
14681470
onClick={() => props.onBulkAction?.(action, selectedRows)}
14691471
data-testid={`bulk-action-${action}`}
14701472
>
1471-
{action.replace(/_/g, ' ').replace(/\b\w/g, c => c.toUpperCase())}
1473+
{formatActionLabel(action)}
14721474
</Button>
14731475
))}
14741476
</div>

0 commit comments

Comments
 (0)