Feat: form field programmatic errors #416
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughButton decoration in the date picker now falls back to the theme when not provided. ShadFormBuilderFieldState gains programmatic custom-error support (new getters, invalidate, validate behavior, reset clears custom error). A golden test was added to assert programmatic error display for ShadInputFormField. Changes
Sequence Diagram(s)sequenceDiagram
actor App as App/Test
participant Field as ShadFormBuilderFieldState
participant Base as Base Validation Logic
App->>Field: invalidate("This field is required")
Field->>Field: _customErrorText = "This field is required"
opt revalidate
Field->>Base: validate()
Base-->>Field: baseError/baseValid
end
Field-->>App: hasError/errorText reflect composite state
App->>Field: validate(clearCustomError: true)
alt clearCustomError == true
Field->>Field: _customErrorText = null
end
Field->>Base: validate()
Base-->>Field: baseError/baseValid
Field-->>App: returns (baseValid && _customErrorText == null)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
PR is finished and ready for review. |
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances form field validation by adding support for programmatic error management alongside existing validation errors. It allows developers to set custom error messages on form fields and provides better granular control over error states.
- Added programmatic error management to
ShadFormBuilderFieldStatewith custom error text support and new validation methods - Introduced comprehensive test coverage for programmatic error functionality in input form fields
- Improved date picker theming to use fallback button decoration from theme
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/src/components/form/field.dart | Enhanced form field state with programmatic error handling, custom error text management, and improved validation methods |
| test/src/components/input_test.dart | Added widget test to verify programmatic error display functionality with golden file comparison |
| lib/src/components/date_picker.dart | Updated date picker to use theme's button decoration as fallback when none provided |
| if (clearCustomError) { | ||
| setState(() => _customErrorText = null); | ||
| } | ||
| final isValid = super.validate() && !hasError; |
There was a problem hiding this comment.
The validation logic creates a circular dependency. The hasError getter includes both validation errors and custom errors, so this line will always return false when _customErrorText is set, even if the field value itself is valid. Consider using super.validate() && _customErrorText == null instead.
| final isValid = super.validate() && !hasError; | |
| final isValid = super.validate() && _customErrorText == null; |
Copilot uses AI. Check for mistakes.
lib/src/components/form/field.dart
Outdated
|
|
||
| validate( | ||
| clearCustomError: false, | ||
| ); |
There was a problem hiding this comment.
[nitpick] Calling validate() inside invalidate() seems counterintuitive since the purpose is to set an error, not validate. This call with clearCustomError: false appears unnecessary and may cause confusion. Consider removing this call or clarifying its purpose with a comment.
| ); |
Copilot uses AI. Check for mistakes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
lib/src/components/form/field.dart (4)
160-165: Doc nit: clarify and tighten wording for errorTextMinor wording tweak for clarity.
-/// Returns the current error text, -/// As it might be validation error or programmatically set. +/// Returns the current error text. +/// It may come from validation or be set programmatically.
166-170: Simplify hasError check to avoid redundant getter usage
errorText != nullis sufficient but calls the getter; prefer a direct check to avoid recomputation.-bool get hasError => super.hasError || errorText != null; +bool get hasError => super.hasError || _customErrorText != null;
235-243: Defaulting validate() to clear custom errors may be surprising; consider default=falseCalling
Form.of(context).validate()will (by default) clear programmatic errors before validating, which may not be what consumers expect. Recommend not clearing custom errors unless explicitly requested.-bool validate({ - bool clearCustomError = true, -}) { - if (clearCustomError) { - setState(() => _customErrorText = null); - } +bool validate({ + bool clearCustomError = false, +}) { + if (clearCustomError && _customErrorText != null) { + setState(() => _customErrorText = null); + } final isValid = super.validate() && !hasError; return isValid; }If the current default is intentional, please confirm the desired behavior for
ShadForm.of(context)?.validate()so we can document it accordingly.
246-253: Consider a convenience clear method and optional revalidation control
invalidateboth sets the error and revalidates. Two enhancements:
- Add a
clearCustomError()helper.- Optionally allow skipping revalidation to avoid a second setState when not needed.
Add a helper method (outside the selected range):
void clearCustomError() { if (_customErrorText != null) { setState(() => _customErrorText = null); } }Optionally extend
invalidate:-void invalidate(String errorText) { - setState(() => _customErrorText = errorText); - - validate( - clearCustomError: false, - ); -} +void invalidate(String errorText, {bool revalidate = true}) { + setState(() => _customErrorText = errorText); + if (revalidate) { + validate(clearCustomError: false); + } +}test/src/components/input_test.dart (1)
31-60: Programmatic error golden — LGTM; add state assertions for robustnessThe golden covers UI; adding state assertions improves test intent and detects regressions even if goldens drift.
// Programmatically set an error on the field key.currentState!.invalidate('This field is required'); await tester.pumpAndSettle(); + // Sanity-check state to catch regressions beyond the golden image. + expect(key.currentState!.hasError, isTrue); + expect(key.currentState!.errorText, 'This field is required'); + expect(key.currentState!.isValid, isFalse); + // Base value has no validator in this test, so valueIsValid should be true. + expect(key.currentState!.valueIsValid, isTrue); expect( find.byType(ShadInputFormField), matchesGoldenFile('goldens/input_error.png'), );Optional follow-ups:
- Add a test that
reset()clears the programmatic error.- Add a test that clarifies
validate(clearCustomError: X)behavior, especially what happens whenForm.of(context)!.validate()is called without args.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/src/components/goldens/input_error.pngis excluded by!**/*.png
📒 Files selected for processing (3)
lib/src/components/date_picker.dart(1 hunks)lib/src/components/form/field.dart(3 hunks)test/src/components/input_test.dart(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-13T09:57:53.516Z
Learnt from: nank1ro
PR: nank1ro/flutter-shadcn-ui#352
File: docs/src/content/docs/index.md:129-135
Timestamp: 2025-05-13T09:57:53.516Z
Learning: The shadcn_ui package uses `WidgetStatePropertyAll` in theme configurations (such as `ScrollbarThemeData`) instead of Flutter's native `MaterialStatePropertyAll`. This is intentional and correct according to the package maintainer.
Applied to files:
lib/src/components/date_picker.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter test
🔇 Additional comments (6)
lib/src/components/date_picker.dart (1)
1165-1166: Theme fallback for button decoration — LGTMUsing the theme’s
datePickerTheme.buttonDecorationwhenwidget.buttonDecorationis null aligns with the rest of the theming strategy and keeps behavior consistent.lib/src/components/form/field.dart (4)
137-137: Private custom error store — LGTMIntroducing
_customErrorTextas the single source of truth for programmatic errors is clear and contained.
171-175: isValid override — LGTMCombining base validity with absence of custom error is correct and expected.
176-181: Expose base-only validity/error — LGTM
valueIsValid/valueHasErrorcleanly separate base validator results from programmatic error state.
223-226: reset clears custom error — LGTMClearing
_customErrorTexton reset is consistent. Thanks for guarding setState behind a null check.test/src/components/input_test.dart (1)
5-7: Imports for form field testing — LGTM
|
@nank1ro, the PR is ready. Could you please review it? 😇🙏 |
There was a problem hiding this comment.
Thanks for the PR @omaralmgerbie 🙏 however it is completely useless now that we have forceErrorText
ShadInputFormField(
forceErrorText: 'Forced error text',
...
),Feel free to close it if you agree
This pull request introduces enhancements to the form field components, particularly improving the management and validation of errors programmatically. It also adds a new test to ensure that programmatically set errors are correctly displayed in the UI. Additionally, it includes a minor improvement to the date picker button's theming.
Form field error handling improvements:
ShadFormBuilderFieldStatevia a new_customErrorTextproperty andinvalidatemethod. The error state now considers both validation and custom errors, and new getters (errorText,hasError,isValid,valueIsValid,valueHasError) provide more granular state information. Thevalidatemethod can optionally clear custom errors, andresetnow clears custom errors as well. [1] [2] [3]Testing enhancements:
input_test.dartto verify that programmatically setting an error onShadInputFormFielddisplays the correct error UI, matching a golden file. [1] [2]UI theming improvements:
ShadDatePickerto use the theme's button decoration as a fallback if none is provided, ensuring more consistent styling.Pre-launch Checklist
///).If you need help, consider asking for advice on Discord.
Summary by CodeRabbit
No breaking API changes; existing behavior remains compatible.