fix: validate non-empty text before saving in text editor#506
Open
bhavjsh wants to merge 3 commits into
Open
Conversation
The text editor's confirm button exported and saved the canvas unconditionally, so an empty or whitespace-only text box was saved as a blank image. Guard the confirm action against empty input and surface the problem to the user. - Reject empty/whitespace-only text on confirm instead of saving - Show an inline field error and a warning SnackBar on failed confirm - Dim the confirm icon while the text is empty and clear the error as valid text is typed - Autofocus the field and enable sentence capitalization - Add localized strings and widget tests for the validation flow
Contributor
Reviewer's GuideThis PR adds front-end validation to the text-fit editor so that empty or whitespace-only text cannot be saved, surfaced via inline field errors, a warning snackbar, and updated button state, with tests and localization strings to cover the new behavior. Sequence diagram for text_fit_editor save validationsequenceDiagram
actor User
participant TextFitEditorState
participant ScaffoldMessenger
participant Navigator
User->>TextFitEditorState: IconButton.onPressed
activate TextFitEditorState
TextFitEditorState->>TextFitEditorState: _handleConfirm(canvasSize, appLocalizations)
alt _isTextEmpty
TextFitEditorState->>TextFitEditorState: setState(_errorText = appLocalizations.emptyTextError)
TextFitEditorState->>ScaffoldMessenger: hideCurrentSnackBar()
TextFitEditorState->>ScaffoldMessenger: showSnackBar(SnackBar(appLocalizations.emptyTextWarning))
TextFitEditorState-->>User: save blocked
else text not empty
TextFitEditorState->>TextFitEditorState: _export(canvasSize)
TextFitEditorState->>Navigator: pop(context, bytes)
end
deactivate TextFitEditorState
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
onChangedhandler only callssetStatewhen_errorTextis non-null, so_isTextEmptyand the check icon’s dimmed state won’t update as the user types once there is no error; consider always callingsetStateon change and clearing_errorTextconditionally inside it. - Instead of conditionally changing the
enabledBorderbased on_errorText, you can use the built-inerrorBorder/focusedErrorBorderfields onInputDecorationto keep the border configuration clearer and more consistent. - The hard-coded red color for the error border could be replaced with a color from the theme (e.g.
Theme.of(context).colorScheme.error) to align with app-wide styling and make dark/light mode behavior consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `onChanged` handler only calls `setState` when `_errorText` is non-null, so `_isTextEmpty` and the check icon’s dimmed state won’t update as the user types once there is no error; consider always calling `setState` on change and clearing `_errorText` conditionally inside it.
- Instead of conditionally changing the `enabledBorder` based on `_errorText`, you can use the built-in `errorBorder`/`focusedErrorBorder` fields on `InputDecoration` to keep the border configuration clearer and more consistent.
- The hard-coded red color for the error border could be replaced with a color from the theme (e.g. `Theme.of(context).colorScheme.error`) to align with app-wide styling and make dark/light mode behavior consistent.
## Individual Comments
### Comment 1
<location path="lib/view/text_fit_editor.dart" line_range="193-197" />
<code_context>
borderRadius: BorderRadius.circular(12),
borderSide: BorderSide.none,
),
+ enabledBorder: OutlineInputBorder(
+ borderRadius: BorderRadius.circular(12),
+ borderSide: _errorText == null
+ ? BorderSide.none
+ : const BorderSide(color: Colors.red, width: 1),
+ ),
),
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using `enabledBorder` for error styling may lead to inconsistent visuals in focused/error states.
Because `_errorText` is set, Flutter will use `errorBorder` / `focusedErrorBorder` in addition to `enabledBorder`. Only changing `enabledBorder` means the border can differ between focused and unfocused error states. Define matching `errorBorder` and `focusedErrorBorder` with the red outline, and keep `enabledBorder` for the non-error state to ensure consistent styling.
Suggested implementation:
```
style: const TextStyle(fontSize: 14),
decoration: InputDecoration(
hintText: appLocalizations.enterTextHint,
errorText: _errorText,
contentPadding: const EdgeInsets.symmetric(
horizontal: 12, vertical: 10),
border: OutlineInputBorder(
borderRadius: BorderRadius.circular(12),
borderSide: BorderSide.none,
),
enabledBorder: OutlineInputBorder(
borderRadius: BorderRadius.circular(12),
borderSide: BorderSide.none,
),
errorBorder: OutlineInputBorder(
borderRadius: BorderRadius.circular(12),
borderSide: const BorderSide(color: Colors.red, width: 1),
),
focusedErrorBorder: OutlineInputBorder(
borderRadius: BorderRadius.circular(12),
borderSide: const BorderSide(color: Colors.red, width: 1),
),
```
If your design requires a visible border when the field is focused and not in an error state, you may also want to define `focusedBorder` with a non-error style (e.g., a primary-color outline), parallel to the `enabledBorder` definition.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ditor Address review feedback on the empty-text validation: - Replace the conditional enabledBorder with dedicated errorBorder and focusedErrorBorder so the error outline is consistent across focused and unfocused states - Use Theme.of(context).colorScheme.error instead of a hard-coded red so the field aligns with app-wide light/dark styling - Simplify the onChanged guard now that the border no longer depends on it
Contributor
Build StatusBuild successful. APKs to test: https://github.com/fossasia/magic-epaper-app/actions/runs/28821902004/artifacts/8121390886. ScreenshotsAndroid Screenshots
iPhone Screenshots
iPad Screenshots
|
-1_display_selection.png?raw=true)
-2_sidebar.png?raw=true)
-3_ndef_screen.png?raw=true)
-4_filter_selection.png?raw=true)
-5_barcode_screen.png?raw=true)
-6_Barcode_added.png?raw=true)
-7_Templates_screen.png?raw=true)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #502
Changes
Recordings
WhatsApp.Video.2026-07-07.at.1.41.51.AM.mp4
Checklist:
constants.dartor localization files instead of hard-coded values.dart formator the IDE formatter.flutter analyzeand tests run influtter test.Summary by Sourcery
Validate non-empty text before allowing saves in the text editor and improve the editor’s UX around empty input.
New Features:
Bug Fixes:
Enhancements:
Tests: