feat(admin): add EntityForm component with core components#472
feat(admin): add EntityForm component with core components#472husamettinarabaci merged 3 commits intodevelopfrom
Conversation
…tion - Implement EntityForm component with dynamic field rendering - Add support for multiple field types (text, email, number, select, checkbox, switch, etc.) - Implement field validation with customizable rules (required, min, max, pattern) - Add form sections for organized field grouping - Include loading and error states with user feedback - Enhance Button component with type prop (button, submit, reset) - Update Alert success icon from 'v' to '✓' for better visual clarity - Add comprehensive Storybook stories for EntityForm with multiple examples
husamettinarabaci
left a comment
There was a problem hiding this comment.
Code Review - Request Changes
Thank you for the comprehensive EntityForm implementation! The component is well-structured and follows most of our standards. However, there are critical embedded string violations that need to be addressed before merge.
❌ Blocking Issues - AGENTS.md Violations
🚨 Issue: Hardcoded Validation Messages (10 instances)
Location: EntityForm.svelte lines 379-448 in validateField() function
AGENTS.md Rule Violation:
❌ NEVER hardcode strings in component implementations: Validation messages
Found Hardcoded Strings:
- Line 379:
return \${field.label} is required`;` - Line 396:
return \${field.label} must be at least ${rules.min}`;` - Line 400:
return \${field.label} must be at least ${rules.min} characters`;` - Line 411:
return \${field.label} must be at most ${rules.max}`;` - Line 415:
return \${field.label} must be at most ${rules.max} characters`;` - Line 425:
return \${field.label} format is invalid`;` - Line 434:
return \${field.label} must be a valid email address`;` - Line 440:
return \${field.label} must be a valid URL`;` - Line 444:
return \${field.label} must be a valid number`;` - Line 448:
return \${field.label} must be a valid date`;`
Why This Matters:
- Prevents internationalization (i18n)
- Prevents message customization by users
- Violates project architecture standards
- Listed explicitly in AGENTS.md forbidden list
🚨 Issue: Hardcoded Error Message (1 instance)
Location: EntityForm.svelte lines 831 and 963
<Text text={\`Unknown field type: ${field.type}\`} variant="error" />AGENTS.md Rule Violation:
❌ NEVER hardcode strings in component implementations: Error messages
✅ Required Changes
1. Add Validation Message Template Props
Add these props to the Props interface:
interface Props {
// ... existing props
/**
* Validation message templates (support {field}, {min}, {max}, {type} placeholders)
*/
/**
* Required field validation message
* @default '{field} is required'
*/
requiredMessage?: string;
/**
* Minimum length validation message
* @default '{field} must be at least {min} characters'
*/
minLengthMessage?: string;
/**
* Minimum value validation message
* @default '{field} must be at least {min}'
*/
minValueMessage?: string;
/**
* Maximum length validation message
* @default '{field} must be at most {max} characters'
*/
maxLengthMessage?: string;
/**
* Maximum value validation message
* @default '{field} must be at most {max}'
*/
maxValueMessage?: string;
/**
* Invalid format validation message
* @default '{field} format is invalid'
*/
invalidFormatMessage?: string;
/**
* Invalid email validation message
* @default '{field} must be a valid email address'
*/
invalidEmailMessage?: string;
/**
* Invalid URL validation message
* @default '{field} must be a valid URL'
*/
invalidUrlMessage?: string;
/**
* Invalid number validation message
* @default '{field} must be a valid number'
*/
invalidNumberMessage?: string;
/**
* Invalid date validation message
* @default '{field} must be a valid date'
*/
invalidDateMessage?: string;
/**
* Unknown field type error message
* @default 'Unknown field type: {type}'
*/
unknownFieldTypeMessage?: string;
}2. Add Default Values in Props Destructuring
const {
// ... existing props
requiredMessage = '{field} is required',
minLengthMessage = '{field} must be at least {min} characters',
minValueMessage = '{field} must be at least {min}',
maxLengthMessage = '{field} must be at most {max} characters',
maxValueMessage = '{field} must be at most {max}',
invalidFormatMessage = '{field} format is invalid',
invalidEmailMessage = '{field} must be a valid email address',
invalidUrlMessage = '{field} must be a valid URL',
invalidNumberMessage = '{field} must be a valid number',
invalidDateMessage = '{field} must be a valid date',
unknownFieldTypeMessage = 'Unknown field type: {type}',
// ...
}: Props = $props();3. Add Template Replacement Utility Function
Add this function after props destructuring:
/**
* Format message template with replacements
* Replaces {key} placeholders with values from replacements object
*/
function formatMessage(template: string, replacements: Record<string, any>): string {
return template.replace(/{(\w+)}/g, (match, key) =>
replacements[key] !== undefined ? String(replacements[key]) : match
);
}4. Update validateField() Function
Replace all hardcoded return statements with formatted messages:
// Line 379 - Required validation
// OLD: return \`${field.label} is required\`;
return formatMessage(requiredMessage, { field: field.label });
// Line 396 - Min value
// OLD: return \`${field.label} must be at least ${rules.min}\`;
return formatMessage(minValueMessage, { field: field.label, min: rules.min });
// Line 400 - Min length
// OLD: return \`${field.label} must be at least ${rules.min} characters\`;
return formatMessage(minLengthMessage, { field: field.label, min: rules.min });
// Line 411 - Max value
// OLD: return \`${field.label} must be at most ${rules.max}\`;
return formatMessage(maxValueMessage, { field: field.label, max: rules.max });
// Line 415 - Max length
// OLD: return \`${field.label} must be at most ${rules.max} characters\`;
return formatMessage(maxLengthMessage, { field: field.label, max: rules.max });
// Line 425 - Invalid format
// OLD: return \`${field.label} format is invalid\`;
return formatMessage(invalidFormatMessage, { field: field.label });
// Line 434 - Invalid email
// OLD: return \`${field.label} must be a valid email address\`;
return formatMessage(invalidEmailMessage, { field: field.label });
// Line 440 - Invalid URL
// OLD: return \`${field.label} must be a valid URL\`;
return formatMessage(invalidUrlMessage, { field: field.label });
// Line 444 - Invalid number
// OLD: return \`${field.label} must be a valid number\`;
return formatMessage(invalidNumberMessage, { field: field.label });
// Line 448 - Invalid date
// OLD: return \`${field.label} must be a valid date\`;
return formatMessage(invalidDateMessage, { field: field.label });5. Update Unknown Field Type Error (Lines 831, 963)
<!-- OLD -->
<Text text={\`Unknown field type: ${field.type}\`} variant="error" />
<!-- NEW -->
<Text text={formatMessage(unknownFieldTypeMessage, { type: field.type })} variant="error" />📝 Example Usage After Fix
Users will be able to customize validation messages:
<EntityForm
fields={userFields}
requiredMessage="{field} es obligatorio"
invalidEmailMessage="{field} debe ser un email válido"
/>✅ What's Already Great
- ✅ Excellent TypeScript usage (no
anytypes) - ✅ Perfect component composition (uses library components)
- ✅ Comprehensive Storybook stories (12 stories)
- ✅ Clean Svelte 5 patterns ($props, $state, $derived)
- ✅ Static DaisyUI classes (no dynamic interpolation)
- ✅ Good accessibility support
- ✅ Proper validation logic
- ✅ Button and Alert enhancements are appropriate
📚 Reference
AGENTS.md Section: "No Hardcoded Strings - Externalize All Text Content"
⚠️ CRITICAL RULE: Component implementations MUST NOT contain hardcoded/embedded strings. All user-facing text must be passed as props for localization and flexibility.
Thank you for your contribution! Once these embedded string issues are resolved, this will be an excellent addition to the library. 🚀
- Add 12 customizable validation message props with template placeholder support
- Add formatMessage() utility for {field}, {min}, {max}, {type} placeholder replacement
- Externalize all hardcoded validation messages for full i18n readiness
- Fix min/max validation to work with all field types (password, email, tel, url, etc.)
- Add submitErrorMessage prop for generic form submission errors
- All messages support customization while maintaining sensible English defaults
- Total: 24 text props externalized (12 validation + 12 UI/accessibility labels)
husamettinarabaci
left a comment
There was a problem hiding this comment.
Code Review - PR #472
Thank you for this comprehensive implementation of the EntityForm component! The overall quality is excellent, with great adherence to AGENTS.md guidelines. However, I've identified 2 issues that need to be addressed before merging.
🚨 Critical Issue - Must Fix
1. Hardcoded String in FormSection.svelte (Line 252)
Location: hexawebshare/src/components/admin/forms/FormSection.svelte:252
Issue: The string "Loading form section" is hardcoded in the component implementation, violating the No Hardcoded Strings rule from AGENTS.md (lines 549-696).
<!-- Current code (INCORRECT) -->
<div
class="flex items-center justify-center py-8"
role="status"
aria-label="Loading form section"
>
<Spinner size="md" />
</div>Required Fix:
Add a loadingLabel prop with a default value:
interface Props {
// ... existing props ...
/**
* Accessible label for loading state
* @default 'Loading form section'
*/
loadingLabel?: string;
}
const {
// ... existing destructuring ...
loadingLabel = 'Loading form section',
// ...
}: Props = $props();Then use it in the template:
<div
class="flex items-center justify-center py-8"
role="status"
aria-label={loadingLabel}
>
<Spinner size="md" />
</div>Why this matters: Component implementations must support localization. All user-facing text must be passed as props to enable i18n support.
⚠️ AGENTS.md Violation - Story Count Limit
2. Too Many Variant Stories in EntityForm.stories.svelte
Issue: The story file contains 11 variant stories, exceeding AGENTS.md's maximum of 10 variant stories (AGENTS.md lines 713-720).
Current Stories:
- Create Mode
- Edit Mode
- With Validation
- Loading State
- Error State
- External Success Message
- Disabled State
- Bordered Variant
- Card Variant
- With Reset Button
- Complex Entity Example ← This exceeds the limit
- Playground ← (Counted separately)
AGENTS.md Rule (line 714):
✅ Maximum 10 stories per component (excluding Playground)
Required Fix: Remove 1 variant story to comply with the 10-story maximum.
Recommendation: Remove the "With Validation" story since validation is already demonstrated in the "Create Mode" story (all fields have validation rules defined in the userFields array).
✅ What's Excellent
Component Composition - Perfect ⭐
- Correctly uses library primitives (Button, Input, Select, etc.)
- No raw HTML for interactive elements
- Follows AGENTS.md compositional architecture perfectly
String Externalization - Excellent ⭐
- 24 text props externalized with template placeholder support
formatMessage()utility with{field},{min},{max},{type}placeholders- EntityForm.svelte has ZERO hardcoded strings (perfect!)
- All validation messages customizable
CSS/Styling - Compliant ⭐
- Static DaisyUI classes only (no dynamic interpolation)
- No custom CSS or fonts
- Proper use of
$derivedfor conditional classes
TypeScript - High Quality ⭐
- Comprehensive interfaces (
EntityField,ValidationRule,Props) - No
anytypes - Excellent JSDoc documentation
Git Workflow - Compliant ⭐
- Branch:
feat/add-entity-form-component(correct format) - Commits follow conventional format
- All CI checks passing
📋 Summary
Approval Status: Request Changes
Issues to Fix:
- ✅ CRITICAL: Externalize hardcoded string in FormSection.svelte (line 252)
- ✅ REQUIRED: Remove 1 variant story to comply with max 10 limit
Overall Quality: 8.5/10 - Excellent work with 2 minor issues
Once these issues are addressed, this PR will be ready to merge. The implementation is otherwise production-ready and sets a great example for component composition and i18n support.
Reference: AGENTS.md lines 549-696 (String Externalization), 708-815 (Storybook Requirements)
- Add loadingLabel prop to FormSection component with default value - Remove duplicate "With Validation" story from EntityForm to comply with max 10 stories limit - Fixes AGENTS.md violations: string externalization and story count requirements Related to PR #472 code review feedback
Pull Request
📄 Summary
This PR introduces a comprehensive
EntityFormcomponent for the admin module, enabling dynamic form generation based on field definitions. The component supports multiple field types, validation rules, form sections, and both create/edit modes. Additionally, it includes enhancements to theButtoncomponent (type prop support) andAlertcomponent (improved success icon).🧩 Affected Module(s)
Mark the modules impacted by this PR:
✅ Checklist
Before submitting, make sure you've completed the following:
🔗 Related Issues
Closes ##223