Skip to content

feat: Replace hardcoded cell formatters with @object-ui/fields widgets in object-agrid #391

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.

Sign up for GitHub

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

Merged
hotlong merged 5 commits into from
Feb 7, 2026
Merged

feat: Replace hardcoded cell formatters with @object-ui/fields widgets in object-agrid #391

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a52d526
Initial plan
Copilot Feb 6, 2026
b35e529
feat: Integrate @object-ui/fields widgets into object-agrid cell rend…
Copilot Feb 6, 2026
d88c02d
chore: Clean up unused imports and variables
Copilot Feb 6, 2026
00d17dd
fix: Address code review feedback - improve type safety and use DOM e…
Copilot Feb 6, 2026
97ae174
fix: Address code review feedback - optimize performance, improve ref…
Copilot Feb 6, 2026
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
1 change: 1 addition & 0 deletions packages/plugin-aggrid/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"dependencies": {
"@object-ui/components": "workspace:*",
"@object-ui/core": "workspace:*",
"@object-ui/fields": "workspace:*",
"@object-ui/react": "workspace:*",
"@object-ui/types": "workspace:*",
"@object-ui/data-objectstack": "workspace:*"
Expand Down
237 changes: 68 additions & 169 deletions packages/plugin-aggrid/src/ObjectAgGridImpl.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@ import type {
StatusPanelDef,
GetContextMenuItemsParams,
MenuItemDef,
IServerSideDatasource,
IServerSideGetRowsParams
} from 'ag-grid-community';
import type { DataSource, FieldMetadata, ObjectSchemaMetadata } from '@object-ui/types';
import type { FieldMetadata, ObjectSchemaMetadata } from '@object-ui/types';
import type { ObjectAgGridImplProps } from './object-aggrid.types';
import { FIELD_TYPE_TO_FILTER_TYPE } from './object-aggrid.types';
import { createFieldCellRenderer, createFieldCellEditor } from './field-renderers';

/**
* ObjectAgGridImpl - Metadata-driven AG Grid implementation
Expand Down Expand Up @@ -61,7 +60,6 @@ export default function ObjectAgGridImpl({
const [error, setError] = useState<Error | null>(null);
const [objectSchema, setObjectSchema] = useState<ObjectSchemaMetadata | null>(null);
const [rowData, setRowData] = useState<any[]>([]);
const [totalCount, setTotalCount] = useState(0);

// Fetch object metadata
useEffect(() => {
Expand Down Expand Up @@ -115,7 +113,6 @@ export default function ObjectAgGridImpl({

const result = await dataSource.find(objectName, queryParams);
setRowData(result.data || []);
setTotalCount(result.total || 0);
callbacks?.onDataLoaded?.(result.data || []);
} catch (err) {
const error = err instanceof Error ? err : new Error(String(err));
Expand Down Expand Up @@ -416,15 +413,6 @@ export default function ObjectAgGridImpl({
);
}

/**
* Escape HTML to prevent XSS attacks
*/
function escapeHtml(text: string): string {
const div = document.createElement('div');
div.textContent = text;
return div.innerHTML;
}

/**
* Get filter type based on field metadata
*/
Expand All @@ -438,166 +426,77 @@ function getFilterType(field: FieldMetadata): string | boolean {

/**
* Apply field type-specific formatting to column definition
* Uses field widgets from @object-ui/fields for consistent rendering
*/
function applyFieldTypeFormatting(colDef: ColDef, field: FieldMetadata): void {
switch (field.type) {
case 'boolean':
colDef.cellRenderer = (params: any) => {
if (params.value === true) return '✓ Yes';
if (params.value === false) return '✗ No';
return '';
};
break;

case 'currency':
colDef.valueFormatter = (params: any) => {
if (params.value == null) return '';
const currency = (field as any).currency || 'USD';
const precision = (field as any).precision || 2;
return new Intl.NumberFormat('en-US', {
style: 'currency',
currency,
minimumFractionDigits: precision,
maximumFractionDigits: precision,
}).format(params.value);
};
break;

case 'percent':
colDef.valueFormatter = (params: any) => {
if (params.value == null) return '';
const precision = (field as any).precision || 2;
return `${(params.value * 100).toFixed(precision)}%`;
};
break;

case 'date':
colDef.valueFormatter = (params: any) => {
if (!params.value) return '';
try {
const date = new Date(params.value);
if (isNaN(date.getTime())) return '';
return date.toLocaleDateString();
} catch {
return '';
}
};
break;

case 'datetime':
colDef.valueFormatter = (params: any) => {
if (!params.value) return '';
try {
const date = new Date(params.value);
if (isNaN(date.getTime())) return '';
return date.toLocaleString();
} catch {
return '';
}
};
break;

case 'time':
colDef.valueFormatter = (params: any) => {
if (!params.value) return '';
return params.value;
};
break;

case 'email':
colDef.cellRenderer = (params: any) => {
if (!params.value) return '';
const escaped = escapeHtml(params.value);
return `<a href="mailto:${escaped}" class="text-blue-600 hover:underline">${escaped}</a>`;
};
break;

case 'url':
colDef.cellRenderer = (params: any) => {
if (!params.value) return '';
const escaped = escapeHtml(params.value);
return `<a href="${escaped}" target="_blank" rel="noopener noreferrer" class="text-blue-600 hover:underline">${escaped}</a>`;
};
break;

case 'phone':
colDef.cellRenderer = (params: any) => {
if (!params.value) return '';
const escaped = escapeHtml(params.value);
return `<a href="tel:${escaped}" class="text-blue-600 hover:underline">${escaped}</a>`;
};
break;

case 'select':
colDef.valueFormatter = (params: any) => {
if (!params.value) return '';
const options = (field as any).options || [];
const option = options.find((opt: any) => opt.value === params.value);
return option?.label || params.value;
};
break;

case 'lookup':
case 'master_detail':
colDef.valueFormatter = (params: any) => {
if (!params.value) return '';
// Handle lookup values - could be an object or just an ID
if (typeof params.value === 'object') {
return params.value.name || params.value.label || params.value.id || '';
}
return String(params.value);
};
break;
// Define field types that should use field widgets for rendering
const fieldWidgetTypes = [
'text', 'textarea', 'number', 'currency', 'percent',
'boolean', 'select', 'date', 'datetime', 'time',
'email', 'phone', 'url', 'password', 'color',
'rating', 'image', 'avatar', 'lookup', 'slider', 'code'
];

// Use field widget renderer if the type is supported
if (fieldWidgetTypes.includes(field.type)) {
colDef.cellRenderer = createFieldCellRenderer(field);

// Add cell editor for editable fields
if (colDef.editable) {
colDef.cellEditor = createFieldCellEditor(field);

case 'number': {
const precision = (field as any).precision;
if (precision !== undefined) {
// Configure editor based on field type
if (['date', 'datetime', 'select', 'lookup', 'color'].includes(field.type)) {
colDef.cellEditorPopup = true;
}
}
} else {
// Fallback to simple rendering for unsupported types
switch (field.type) {
case 'lookup':
case 'master_detail':
colDef.valueFormatter = (params: any) => {
if (params.value == null) return '';
return Number(params.value).toFixed(precision);
if (!params.value) return '';
// Handle lookup values - could be an object or just an ID
if (typeof params.value === 'object') {
return params.value.name || params.value.label || params.value.id || '';
}
return String(params.value);
};
break;
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field type 'lookup' appears in both the fieldWidgetTypes array (line 437) and in the fallback switch statement (line 456). This creates redundant code where the lookup type will use the field widget renderer but the fallback case will never be reached. Remove the 'lookup' and 'master_detail' cases from the fallback switch statement since they're already handled by the field widget renderer.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 97ae174. Removed the redundant 'lookup' case from the fallback switch since it's already handled by the field widget renderer in line 437.


case 'object':
colDef.cellRenderer = () => {
const span = document.createElement('span');
span.className = 'text-gray-500 italic';
span.textContent = '[Object]';
return span;
};
break;

case 'vector':
colDef.cellRenderer = () => {
const span = document.createElement('span');
span.className = 'text-gray-500 italic';
span.textContent = '[Vector]';
return span;
};
break;

case 'grid':
colDef.cellRenderer = () => {
const span = document.createElement('span');
span.className = 'text-gray-500 italic';
span.textContent = '[Grid]';
return span;
};
break;

default:
// Default text rendering
colDef.valueFormatter = (params: any) => {
return params.value != null ? String(params.value) : '';
};
}
break;
}

case 'color':
colDef.cellRenderer = (params: any) => {
if (!params.value) return '';
const escaped = escapeHtml(params.value);
return `<div class="flex items-center gap-2">
<div style="width: 16px; height: 16px; background-color: ${escaped}; border: 1px solid #ccc; border-radius: 2px;"></div>
<span>${escaped}</span>
</div>`;
};
break;

case 'rating':
colDef.cellRenderer = (params: any) => {
if (params.value == null) return '';
const max = (field as any).max || 5;
const stars = '⭐'.repeat(Math.min(params.value, max));
return stars;
};
break;

case 'image':
colDef.cellRenderer = (params: any) => {
if (!params.value) return '';
const url = typeof params.value === 'string' ? params.value : params.value.url;
if (!url) return '';
const escapedUrl = escapeHtml(url);
return `<img src="${escapedUrl}" alt="" style="width: 40px; height: 40px; object-fit: cover; border-radius: 4px;" />`;
};
break;

case 'avatar':
colDef.cellRenderer = (params: any) => {
if (!params.value) return '';
const url = typeof params.value === 'string' ? params.value : params.value.url;
if (!url) return '';
const escapedUrl = escapeHtml(url);
return `<img src="${escapedUrl}" alt="" style="width: 32px; height: 32px; object-fit: cover; border-radius: 50%;" />`;
};
break;
}
}
Loading
Loading