Fix: updated refresh message format and added missing UI labels#333
Open
Aryan-Singla wants to merge 4 commits intofossasia:mainfrom
Open
Fix: updated refresh message format and added missing UI labels#333Aryan-Singla wants to merge 4 commits intofossasia:mainfrom
Aryan-Singla wants to merge 4 commits intofossasia:mainfrom
Conversation
Contributor
Reviewer's GuideUpdates localization keys and strings to support new refresh mode messaging and missing UI labels, while cleaning up locale handling and aligning the image editor UI with the new text and import labels. Class diagram for updated AppLocalizations hierarchy and new refresh mode labelsclassDiagram
class AppLocalizations {
<<abstract>>
+String appName
+String aboutUsDescription
+String library
+String text
+String processingImages
+String refreshModeInfo
+String fullRefreshInfo
+String fullRefreshDescription
+String partialRefreshInfo
+String partialRefreshDescription
+String longPressForInfo
+String chooseImageFromGallery
}
class AppLocalizationsEn {
+String appName
+String library
+String text
+String processingImages
+String refreshModeInfo
+String fullRefreshInfo
+String fullRefreshDescription
+String partialRefreshInfo
+String partialRefreshDescription
+String longPressForInfo
}
class AppLocalizationsHe {
+String developedBy
+String fossasiaContributors
+String aboutUs
+String scanningForNfcTag
+String rawPrefix
+String recordPrefix
+String rawPayloadLabel
+String readOperationCompleted
+String record
+String recordsWritten
+String recordsFound
+String readNdefTags
+String reading
+String readNfcTag
+String refreshNfcStatus
+String readOperationFailed
+String library
+String text
+String scanBarcode
+String readyToFlash
+String resultsCleared
+String saveImagesFromEditor
+String renameImage
+String rename
+String saveImage
+String saveFilteredImageToLibrary
+String renamingImage
+String savingImage
+String refreshingDisplay
+String readNfcTags
+String productImage
+String productName
+String quantitySize
+String qrCodeData
+String profilePhoto
+String productDetails
+String required
+String productImageIn
+String productImageSelected
+String processingImages
+String refreshModeInfo
+String fullRefreshInfo
+String fullRefreshDescription
+String partialRefreshInfo
+String partialRefreshDescription
+String longPressForInfo
}
class AppLocalizationsHi {
+String appName
+String aboutUsDescription
+String library
+String text
+String processingImages
+String refreshModeInfo
+String fullRefreshInfo
+String fullRefreshDescription
+String partialRefreshInfo
+String partialRefreshDescription
+String longPressForInfo
}
class AppLocalizationsNb {
+String library
+String text
+String processingImages
+String refreshModeInfo
+String fullRefreshInfo
+String fullRefreshDescription
+String partialRefreshInfo
+String partialRefreshDescription
+String longPressForInfo
}
class AppLocalizationsNbNo {
<<removed>>
}
class AppLocalizationsDe
class AppLocalizationsEs
class AppLocalizationsFr
class AppLocalizationsId
class AppLocalizationsJa
class AppLocalizationsPt
class AppLocalizationsRu
class AppLocalizationsUk
class AppLocalizationsVi
class AppLocalizationsZh
AppLocalizations <|-- AppLocalizationsEn
AppLocalizations <|-- AppLocalizationsHe
AppLocalizations <|-- AppLocalizationsHi
AppLocalizations <|-- AppLocalizationsNb
AppLocalizations <|-- AppLocalizationsDe
AppLocalizations <|-- AppLocalizationsEs
AppLocalizations <|-- AppLocalizationsFr
AppLocalizations <|-- AppLocalizationsId
AppLocalizations <|-- AppLocalizationsJa
AppLocalizations <|-- AppLocalizationsPt
AppLocalizations <|-- AppLocalizationsRu
AppLocalizations <|-- AppLocalizationsUk
AppLocalizations <|-- AppLocalizationsVi
AppLocalizations <|-- AppLocalizationsZh
Class diagram for ImageEditor refresh mode message and bottom action labelsclassDiagram
class ImageEditor {
+DisplayDevice device
}
class _ImageEditorState {
-Waveform selectedWaveform
+Widget build(BuildContext context)
+void onWaveformSelected(Waveform waveform)
+void showRefreshModeSnackBar(BuildContext context)
}
class BottomActionMenu {
+ImgLoader imgLoader
+Widget build(BuildContext context)
+Widget _buildActionButton(BuildContext context, IconData icon, String label, Function onTap)
}
class AppLocalizations {
+String fullRefreshSelected
+String waveformSelected
+String imported
}
class ScaffoldMessenger {
+static ScaffoldMessenger of(BuildContext context)
+void showSnackBar(SnackBar snackBar)
}
class SnackBar {
+Widget content
+Color backgroundColor
}
class Text {
+Text(String data)
}
class Icons {
<<static>>
+IconData add_photo_alternate_outlined
}
ImageEditor --> _ImageEditorState
_ImageEditorState --> AppLocalizations : uses
_ImageEditorState --> ScaffoldMessenger : shows
ScaffoldMessenger --> SnackBar : displays
SnackBar --> Text : content
BottomActionMenu --> AppLocalizations : uses
BottomActionMenu --> Icons : uses
_ImageEditorState : showRefreshModeSnackBar()
_ImageEditorState : "if selectedWaveform == null use fullRefreshSelected"
_ImageEditorState : "else use selectedWaveform.name + ' Selected'"
BottomActionMenu : "label = appLocalizations.imported"
File-Level Changes
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 2 issues, and left some high level feedback:
- The updated waveform selection snackbar now hardcodes the "Selected" suffix instead of using a localized string; consider exposing a localization key for this pattern so the whole message remains translatable.
- In the bottom action menu, the import button label was changed from
appLocalizations.importtoappLocalizations.imported, which reads like a past-tense status rather than an action; consider keeping or introducing an action-oriented localization key for this button.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The updated waveform selection snackbar now hardcodes the "Selected" suffix instead of using a localized string; consider exposing a localization key for this pattern so the whole message remains translatable.
- In the bottom action menu, the import button label was changed from `appLocalizations.import` to `appLocalizations.imported`, which reads like a past-tense status rather than an action; consider keeping or introducing an action-oriented localization key for this button.
## Individual Comments
### Comment 1
<location path="lib/view/image_editor.dart" line_range="436" />
<code_context>
? appLocalizations
.fullRefreshSelected
- : "${appLocalizations.waveformSelected} ${_selectedWaveform!.name}",
+ : "${_selectedWaveform!.name} Selected",
),
backgroundColor: colorPrimary,
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid hard‑coding the "Selected" suffix and use a localized string instead.
The previous implementation (`appLocalizations.waveformSelected`) kept this label fully localizable. With the hard‑coded "Selected" suffix, this chip will remain English in non‑English locales. Please either revert to the previous localized pattern (e.g. `"${appLocalizations.waveformSelected} ${_selectedWaveform!.name}"`) or introduce a parameterized localized string so both the waveform name and the "selected" text are translated.
</issue_to_address>
### Comment 2
<location path="lib/l10n/app_localizations_hi.dart" line_range="16" />
<code_context>
@override
String get aboutUsDescription =>
- 'Magic ePaper is an app designed to control and update ePaper displays. The goal is to provide tools for customizing and transferring images, text, and patterns to ePaper screens using NFC. Data transfer from the smartphone to the ePaper hardware is done wirelessly via NFC. The project is built on top of custom firmware and display drivers for seamless communication and efficient image rendering.';
+ 'मैजिक ईपेपर एक ऐसा ऐप है जिसे ईपेपर डिस्प्ले को नियंत्रित और अपडेट करने के लिए डिज़ाइन किया गया है। इसका उद्देश्य एनएफसी का उपयोग करके ईपेपर स्क्रीन पर छवियों, टेक्स्ट और पैटर्न को अनुकूलित और स्थानांतरित करने के लिए उपकरण प्रदान करना है। स्मार्टफोन से ईपेपर हार्डवेयर में डेटा ट्रांसफर एनएफसी के माध्यम से वायरलेस तरीके से किया जाता है। यह प्रोजेक्ट निर्באध संचार और कुशल इमेज रेंडरिंग के लिए कस्टम फ़र्मवेयर और डिस्प्ले ड्राइवरों पर आधारित है।';
@override
</code_context>
<issue_to_address>
**issue (typo):** The Hindi description string appears to contain mixed‑script characters (`निर्באध`).
`निर्באध` mixes Devanagari with (likely) Hebrew characters (`בא`), which will look wrong to users and is probably a copy/paste error. Please replace it with the correct Hindi word (e.g., `निरंतर`, `निर्बाध`, etc.) to avoid confusion.
Suggested implementation:
```
'मैजिक ईपेपर एक ऐसा ऐप है जिसे ईपेपर डिस्प्ले को नियंत्रित और अपडेट करने के लिए डिज़ाइन किया गया है। इसका उद्देश्य एनएफसी का उपयोग करके ईपेपर स्क्रीन पर छवियों, टेक्स्ट और पैटर्न को अनुकूलित और स्थानांतरित करने के लिए उपकरण प्रदान करना है। स्मार्टफोन से ईपेपर हार्डवेयर में डेटा ट्रांसफर एनएफसी के माध्यम से वायरलेस तरीके से किया जाता है। यह प्रोजेक्ट निर्बाध संचार और कुशल इमेज रेंडरिंग के लिए कस्टम फ़र्मवेयर और डिस्प्ले ड्राइवरों पर आधारित है।';
```
The snippet shown includes duplicated declarations around `appName` and `aboutUsDescription` (an extra `String get aboutUsDescription =>` and `String get appName => 'मैजिक ईपेपर';`). If this reflects the real file, those duplicates should be cleaned up to avoid compilation issues.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @override | ||
| String get aboutUsDescription => | ||
| 'Magic ePaper is an app designed to control and update ePaper displays. The goal is to provide tools for customizing and transferring images, text, and patterns to ePaper screens using NFC. Data transfer from the smartphone to the ePaper hardware is done wirelessly via NFC. The project is built on top of custom firmware and display drivers for seamless communication and efficient image rendering.'; | ||
| 'मैजिक ईपेपर एक ऐसा ऐप है जिसे ईपेपर डिस्प्ले को नियंत्रित और अपडेट करने के लिए डिज़ाइन किया गया है। इसका उद्देश्य एनएफसी का उपयोग करके ईपेपर स्क्रीन पर छवियों, टेक्स्ट और पैटर्न को अनुकूलित और स्थानांतरित करने के लिए उपकरण प्रदान करना है। स्मार्टफोन से ईपेपर हार्डवेयर में डेटा ट्रांसफर एनएफसी के माध्यम से वायरलेस तरीके से किया जाता है। यह प्रोजेक्ट निर्באध संचार और कुशल इमेज रेंडरिंग के लिए कस्टम फ़र्मवेयर और डिस्प्ले ड्राइवरों पर आधारित है।'; |
Contributor
There was a problem hiding this comment.
issue (typo): The Hindi description string appears to contain mixed‑script characters (निर्באध).
निर्באध mixes Devanagari with (likely) Hebrew characters (בא), which will look wrong to users and is probably a copy/paste error. Please replace it with the correct Hindi word (e.g., निरंतर, निर्बाध, etc.) to avoid confusion.
Suggested implementation:
'मैजिक ईपेपर एक ऐसा ऐप है जिसे ईपेपर डिस्प्ले को नियंत्रित और अपडेट करने के लिए डिज़ाइन किया गया है। इसका उद्देश्य एनएफसी का उपयोग करके ईपेपर स्क्रीन पर छवियों, टेक्स्ट और पैटर्न को अनुकूलित और स्थानांतरित करने के लिए उपकरण प्रदान करना है। स्मार्टफोन से ईपेपर हार्डवेयर में डेटा ट्रांसफर एनएफसी के माध्यम से वायरलेस तरीके से किया जाता है। यह प्रोजेक्ट निर्बाध संचार और कुशल इमेज रेंडरिंग के लिए कस्टम फ़र्मवेयर और डिस्प्ले ड्राइवरों पर आधारित है।';
The snippet shown includes duplicated declarations around appName and aboutUsDescription (an extra String get aboutUsDescription => and String get appName => 'मैजिक ईपेपर';). If this reflects the real file, those duplicates should be cleaned up to avoid compilation issues.
Member
|
Thanks for the contribution! A process note. We have automatic Copilot PR reviews enabled on this repository. These reviews are only triggered if the contributor has GitHub Copilot enabled and an active license on their own account. Please enable Copilot in your GitHub settings if you have access. In many regions, free licenses are available through educational institutions or developer programs. Enabling Copilot helps us speed up the auto review process and reduces manual review overhead for the core team. |
mariobehling
requested changes
Apr 13, 2026
Member
mariobehling
left a comment
mariobehling
left a comment
There was a problem hiding this comment.
Hi, thanks for the contribution. There are already AI reviews that point out issues. Please address them. If these are not relevant please reply to them.
Member
|
Please also add a screencast for your changes. |
There was a problem hiding this comment.
Pull request overview
Updates image editor UX messaging and localization strings to make refresh-mode feedback consistent and to fill in missing UI labels.
Changes:
- Standardized waveform selection snackbar message formatting to
WaveformName Selected. - Added new localization keys (e.g.,
text, refresh-mode info strings,processingImages) and updated supported locale lookup for Norwegian Bokmål. - Updated UI labels in the image editor bottom action menu (import/editor/text).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/view/image_editor.dart | Updates refresh-mode snackbar text and bottom action menu labels (adds Text label). |
| lib/l10n/app_localizations.dart | Adds new localization getters and removes nb_NO locale specialization from supported locales/lookup. |
| lib/l10n/app_localizations_zh.dart | Adds implementations for new localization getters. |
| lib/l10n/app_localizations_vi.dart | Adds implementations for new localization getters. |
| lib/l10n/app_localizations_uk.dart | Adds implementations for new localization getters. |
| lib/l10n/app_localizations_ru.dart | Adds implementations for new localization getters. |
| lib/l10n/app_localizations_pt.dart | Adds implementations for new localization getters. |
| lib/l10n/app_localizations_nb.dart | Adds implementations for new localization getters and removes nb_NO subclass. |
| lib/l10n/app_localizations_ja.dart | Adds implementations for new localization getters. |
| lib/l10n/app_localizations_id.dart | Adds implementations for new localization getters. |
| lib/l10n/app_localizations_hi.dart | Updates Hindi translations and adds implementations for new localization getters. |
| lib/l10n/app_localizations_he.dart | Updates Hebrew translations and adds implementations for new localization getters. |
| lib/l10n/app_localizations_fr.dart | Adds implementations for new localization getters. |
| lib/l10n/app_localizations_es.dart | Adds implementations for new localization getters. |
| lib/l10n/app_localizations_en.dart | Adds implementations for new localization getters. |
| lib/l10n/app_localizations_de.dart | Adds implementations for new localization getters. |
| lib/l10n/app_en.arb | Adds the text localization key. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
433
to
437
| _selectedWaveform == null | ||
| ? appLocalizations | ||
| .fullRefreshSelected | ||
| : "${appLocalizations.waveformSelected} ${_selectedWaveform!.name}", | ||
| : "${_selectedWaveform!.name} Selected", | ||
| ), |
There was a problem hiding this comment.
The waveform selection snackbar message is now hard-coded ("${_selectedWaveform!.name} Selected"), which bypasses localization and makes it impossible to support languages where the word order differs. Use localized strings (e.g., a new ARB message with a {waveformName} placeholder, or at least combine with an existing localized "Selected" string) so this message can be translated consistently.
| context: context, | ||
| icon: Icons.add_photo_alternate_outlined, | ||
| label: appLocalizations.import, | ||
| label: appLocalizations.imported, |
There was a problem hiding this comment.
This button triggers pickImage(...) (an import action), but the label uses appLocalizations.imported (past tense / source filter). This reads incorrectly before any import happens. Prefer the existing action label localization (e.g. importImageButtonLabel) or introduce a dedicated "Import" label key for this button.
Suggested change
| label: appLocalizations.imported, | |
| label: appLocalizations.importImageButtonLabel, |
Author
|
I have made the required change here is evidence Screen.Recording.2026-04-15.at.11.36.31.PM.mov |
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
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.
For Issue :
Fixes #332
Description :
Summary by Sourcery
Improve localization coverage and refresh mode UX messaging across the image editor and l10n layer.
New Features:
Bug Fixes:
Enhancements: