feat: adding different font styles #1351
Conversation
…gemagic-app into recover-savedbadges
Reviewer's GuideThis PR integrates a Google Fonts–based font picker into the HomeScreen text field for real-time style updates, extends badge editing by loading and updating saved badge files, and strengthens file I/O and data persistence through synchronization and utility classes. Sequence diagram for editing a saved badge with font and effect restorationsequenceDiagram
actor User
participant HomeScreen
participant FileHelper
participant BadgeTextStorage
participant AnimationBadgeProvider
participant SpeedDialProvider
User->>HomeScreen: Open HomeScreen with savedBadgeFilename
HomeScreen->>FileHelper: Load badge JSON from disk
FileHelper-->>HomeScreen: Return badge data
HomeScreen->>BadgeTextStorage: Get original text for badge
BadgeTextStorage-->>HomeScreen: Return original text
HomeScreen->>AnimationBadgeProvider: Set effects and animation mode
HomeScreen->>SpeedDialProvider: Set speed dial value
HomeScreen->>HomeScreen: Set text field to original text
User->>HomeScreen: Edit and save badge
HomeScreen->>SavedBadgeProvider: updateBadgeData(...)
SavedBadgeProvider->>FileHelper: Save updated badge data
SavedBadgeProvider->>BadgeTextStorage: Save updated original text
FileHelper-->>SavedBadgeProvider: Confirm save
BadgeTextStorage-->>SavedBadgeProvider: Confirm save
SavedBadgeProvider-->>HomeScreen: Confirm update
HomeScreen-->>User: Show "Badge Updated Successfully"
Class diagram for new and updated types: FontPickerDialog, BadgeTextStorage, SavedBadgeProviderclassDiagram
class FontPickerDialog {
+String? selectedFont
+ValueChanged<String?> onFontSelected
+build(BuildContext context)
}
class BadgeTextStorage {
<<static>>
+saveOriginalText(String badgeFilename, String originalText)
+getOriginalText(String badgeFilename)
+deleteOriginalText(String badgeFilename)
}
class SavedBadgeProvider {
+applySavedBadgeDataToUI(...)
+updateBadgeData(...)
+getBadgeData(...)
+savedBadgeAnimation(...)
+isSavedBadgeData: bool
+setSavedBadgeDataMap(...)
+setIsSavedBadgeData(...)
}
FontPickerDialog --|> StatelessWidget
SavedBadgeProvider --|> ChangeNotifier
File-Level Changes
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}
There was a problem hiding this comment.
Hey @nope3472 - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `lib/view/homescreen.dart:540` </location>
<code_context>
+ if (state == AppLifecycleState.resumed) {
+ inlineimagecontroller.clear();
+ previousText = '';
+ animationProvider.stopAllAnimations.call(); // If method exists
+ animationProvider.initializeAnimation.call(); // If method exists
+ if (mounted) setState(() {});
+ } else if (state == AppLifecycleState.paused ||
</code_context>
<issue_to_address>
Calling methods with .call() may cause runtime errors if not callable.
Using `.call()` here will throw if these are standard methods. Invoke them directly as methods instead.
</issue_to_address>
### Comment 2
<location> `lib/view/homescreen.dart:270` </location>
<code_context>
+ onChanged: (value) {},
+ controller: inlineimagecontroller,
+ specialTextSpanBuilder: ImageBuilder(),
+ style: _selectedFontFamily != null
+ ? GoogleFonts.getFont(_selectedFontFamily!)
+ : null,
+ decoration: InputDecoration(
+ border: OutlineInputBorder(
</code_context>
<issue_to_address>
No fallback for invalid or unavailable font family.
Validate _selectedFontFamily or add a fallback to avoid exceptions if the font is unsupported.
</issue_to_address>
### Comment 3
<location> `lib/bademagic_module/utils/file_helper.dart:203` </location>
<code_context>
+ /// Save the original text for a badge
+ static Future<void> saveOriginalText(
+ String badgeFilename, String originalText) async {
+ try {
+ // Get the existing text storage or create a new one
+ Map<String, String> textStorage = await _getTextStorage();
</code_context>
<issue_to_address>
Logging uses info level for errors.
Use `logger.e` for error messages to clearly indicate error-level events instead of `logger.i`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| animationProvider.stopAllAnimations.call(); // If method exists | ||
| animationProvider.initializeAnimation.call(); // If method exists |
There was a problem hiding this comment.
issue (bug_risk): Calling methods with .call() may cause runtime errors if not callable.
Using .call() here will throw if these are standard methods. Invoke them directly as methods instead.
lib/view/homescreen.dart
Outdated
| style: _selectedFontFamily != null | ||
| ? GoogleFonts.getFont(_selectedFontFamily!) | ||
| : null, |
There was a problem hiding this comment.
issue: No fallback for invalid or unavailable font family.
Validate _selectedFontFamily or add a fallback to avoid exceptions if the font is unsupported.
| try { | ||
| final path = await _getFilePath(filename); | ||
| final file = File(path); | ||
|
|
||
| if (await file.exists()) { | ||
| final content = await file.readAsString(); | ||
| final dynamic decodedData = jsonDecode(content); | ||
|
|
||
| // Automatically return decoded JSON as a dynamic type | ||
| return decodedData; | ||
| } else { | ||
| logger.d('File not found: $filename'); | ||
| return null; | ||
| } | ||
| return await _badgeFileLock.synchronized(() async { | ||
| if (await file.exists()) { | ||
| final content = await file.readAsString(); | ||
| return jsonDecode(content); | ||
| } else { | ||
| logger.i('File not found: $path'); | ||
| return null; |
There was a problem hiding this comment.
nitpick: Logging uses info level for errors.
Use logger.e for error messages to clearly indicate error-level events instead of logger.i.
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/16571151071/artifacts/3629966034. Screenshots (Android)
Screenshots (iPhone)
Screenshots (iPad)
|
||||||||||||||||||||||||
-1_home_screen.png?raw=true)
-2_text_badge.png?raw=true){kind=icon}
-3_emoji_badge.png?raw=true){kind=icon}
-4_inverted_emoji_badge.png?raw=true){kind=icon}
-5_saved_badges.png?raw=true)
-6_saved_badges_clicked.png?raw=true)
-7_draw_badge.png?raw=true){kind=icon}
|
@nope3472 will you add recording or image for the output ? |
There was a problem hiding this comment.
Please check the following
- Code structure and maintainability: The font-handling logic is directly embedded within core UI or rendering code, making it difficult to maintain or extend. Recommendation:
- Move font-specific logic (e.g., style detection, hex generation) into a dedicated utility or service class.
- Keep the UI code focused on layout and user interaction, not logic processing.
- Lack of inline documentation: There are no comments explaining how the font style logic works, especially for the hex string generation and bitmap handling. Recommendation:
- Add
///comments above all public methods and non-obvious logic blocks. - Document the font support limitations (e.g., resolution, character set constraints).
-
Hardware constraints not accounted for
Based on prior discussion, some fonts clip or render indistinctly due to badge resolution limits. However, these limitations are not surfaced in the code or PR description. Clarify, how could this be handled, e.g. Would it be suitable to add conditionals or UI warnings if unsupported font styles are selected? -
Missing tests: No unit or integration tests are provided to verify the font conversion logic or UI state handling. Recommendation:
- Add test coverage for font selection logic.
- Test fallback behavior if a font cannot be applied.
- PR Description: Add the issue that is fixed into the PR description.
|
hey @Vishveshwara thank for pointing this out i have fixed your concerns and here is how i am using json to save the text- |
yes now the fontstyles are more accurate and gets transferred to the badge correctly |
|
@Vishveshwara there will be some fonts which will not be able to render the characters correctly due to our 11x8 logic. |
|
@AsCress What do you think cause of this ? |
|
@samruddhi-Rahegaonkar yeah will do the changes |
|
@nope3472 here the failing CIs cause is not you code i guess, its something related to CI/CD |
|
@nope3472 @samruddhi-Rahegaonkar This is fine. Just rebase. Seems like the test was canceled due to some reason. |
|
@nope3472 the font Montserrat, Raleway is not working properly please remove it instead of adding multiple font styles lets focus which could be render good on badge. |
|
@nope3472 After saving badges the font is not saving and in saved badges its rendering as normal font. |
|
@nope3472 please make sure that I see the changes related to current issue only there are a lot of changes that are not related to current issue. |
There was a problem hiding this comment.
@nope3472 Please update the branch and clean up the commits to include only changes related to this PR. Once done, we're good to go. Thanks!
There was a problem hiding this comment.
@nope3472 @hpdang this is good as we are starting but the font styles are not rendering properly we need to improve font rendering but for now i think we can go with this as of now but need changes also @nope3472 The message of some characters might get cutoff due to badge size for evry font style seems to be irritating make a toast message little bit small and only for font styles in which characters get actually cut-off.
@hpdang What do you think ?
@nope3472 we should not have the toast message at all, no one would like to use fonts which gets cut off , its better to only support the fonts which are getting rendered properly. |
|
@samruddhi-Rahegaonkar @Vishveshwara okay i will recheck this toastmessage |
@Vishveshwara Absolutely Correct! |
|
@nope3472 Most of the fonts are not rendered properly. |
|
@samruddhi-Rahegaonkar the hexstring that gets generated from the logic i have checked them and when we convert them into 8 bit strings they are generating a image of the characters correctly but the constraints are blocking them |
Summary
Add a font‐picker button to the HomeScreen text field so users can choose and apply Google Fonts in real time.
Changes
FontPickerDialogfor selecting fonts.google_fontspackage.Checklist
constants.dart.flutter analyze&flutter test).images
Summary by Sourcery
Add a font picker to the HomeScreen text field and extend badge persistence and editing capabilities, while improving file handling and data management utilities.
New Features:
Enhancements:
Build:
Chores: