fix: align text and icon in Write NFC Tags#319
Conversation
|
Small 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. The team will review your PR. Thank you for your contribution and cooperation. |
There was a problem hiding this comment.
Pull request overview
This PR primarily targets a UI alignment issue in the “Write NFC Tags” screen by adjusting the behavior of the multiline text input, and it also includes a broader regeneration/update of Flutter-generated artifacts (localizations, plugin registrant) and the dependency lockfile.
Changes:
- Adjusted the “Write NDEF Records” text field to render as single-line by default and expand up to the configured max lines.
- Regenerated localizations to add new strings and adjust supported locale handling (notably Norwegian Bokmål locale handling).
- Updated
pubspec.lockSDK constraints/dependency classification and regenerated the macOS plugin registrant.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pubspec.lock | Updates dependency lock metadata and SDK minimums (Dart/Flutter). |
| macos/Flutter/GeneratedPluginRegistrant.swift | Regenerated macOS plugin registration list (adds mobile_scanner, removes device_info_plus). |
| lib/ndef_screen/widgets/nfc_write_card.dart | Tweaks TextField line behavior to improve initial alignment and expansion. |
| lib/l10n/app_localizations.dart | Adds new localization keys and updates supported locale/lookup logic. |
| lib/l10n/app_localizations_de.dart | Regenerated localization outputs to include new keys. |
| lib/l10n/app_localizations_en.dart | Regenerated localization outputs to include new keys. |
| lib/l10n/app_localizations_es.dart | Regenerated localization outputs to include new keys. |
| lib/l10n/app_localizations_fr.dart | Regenerated localization outputs to include new keys. |
| lib/l10n/app_localizations_he.dart | Regenerated localization outputs (includes multiple updated Hebrew strings + new keys). |
| lib/l10n/app_localizations_hi.dart | Regenerated localization outputs (includes updated Hindi strings + new keys). |
| lib/l10n/app_localizations_id.dart | Regenerated localization outputs to include new keys. |
| lib/l10n/app_localizations_ja.dart | Regenerated localization outputs to include new keys. |
| lib/l10n/app_localizations_nb.dart | Regenerated localization outputs to include new keys and simplify nb locale handling. |
| lib/l10n/app_localizations_pt.dart | Regenerated localization outputs to include new keys. |
| lib/l10n/app_localizations_ru.dart | Regenerated localization outputs to include new keys. |
| lib/l10n/app_localizations_uk.dart | Regenerated localization outputs to include new keys. |
| lib/l10n/app_localizations_vi.dart | Regenerated localization outputs to include new keys. |
| lib/l10n/app_localizations_zh.dart | Regenerated localization outputs to include new keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a70095f to
8b32899
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
lib/ndef_screen/widgets/nfc_write_card.dart:342
- PR description mentions fixing multiline hint alignment via
alignLabelWithHint: true, but this TextField only useshintText(nolabelText) and doesn't setalignLabelWithHint. If alignment is still an issue, either update the PR description to match the actual change, or adjust the relevant InputDecoration/TextField properties that affect hint/prefix icon vertical alignment (sincealignLabelWithHintwon't have any effect here).
return TextField(
onChanged: onChanged,
maxLines: maxLines,
minLines: 1,
obscureText: obscureText,
style: const TextStyle(fontSize: 14),
decoration: InputDecoration(
hintText: hintText,
hintStyle: TextStyle(color: Colors.grey[500], fontSize: 14),
prefixIcon: Icon(prefixIcon, color: colorAccent, size: 20),
border: OutlineInputBorder(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@mariobehling |
rahul31124
left a comment
There was a problem hiding this comment.
Hello @shraavv, I have reviewed and tested the changes the UI looks good! However, please revert the following files: GeneratedPluginRegistrant.swift and pubspec.lock.
Additionally, could you please revert the localization files? They don't seem necessary for this specific fix. If there is an actual issue with the localizations, please open a separate issue and PR so we can address it there.
|
@rahul31124 I really appreciate the review, thanks! The files you mentioned do not need changes, I missed it. I shall be updating the PR soon. |
d143dfb to
5d35691
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @override | ||
| Widget build(BuildContext context) { | ||
| final appLocalizations = AppLocalizations.of(context)!; | ||
| var imgLoader = context.watch<ImageLoader>(); |
There was a problem hiding this comment.
appLocalizations is now declared as a local variable inside build(), but this file still references appLocalizations in other methods (e.g. _exportXbmFiles() and _showRefreshModeInfoDialog()), which will be out of scope and cause compilation errors. Make appLocalizations a late final/field set in didChangeDependencies(), add a getter that returns AppLocalizations.of(context)!, or replace those references to use AppLocalizations.of(context)! within each method.
| import 'package:magicepaperapp/constants/color_constants.dart'; | ||
| import 'package:magicepaperapp/l10n/app_localizations.dart'; | ||
| import '../util/app_logger.dart'; | ||
| import 'package:magicepaperapp/provider/getitlocator.dart'; |
There was a problem hiding this comment.
getitlocator.dart is imported but no longer used in this file after removing the global getIt.get<AppLocalizations>() call. This will trigger an unused import warning (and may fail CI if analysis treats warnings as errors); remove the import or reintroduce its usage intentionally.
| import 'package:magicepaperapp/provider/getitlocator.dart'; |
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| final appLocalizations = AppLocalizations.of(context)!; |
There was a problem hiding this comment.
There is an extra leading space before final appLocalizations = ... which makes indentation inconsistent with the surrounding code and will be caught by dart format. Run the formatter or fix the indentation so it matches the rest of the file.
| final appLocalizations = AppLocalizations.of(context)!; | |
| final appLocalizations = AppLocalizations.of(context)!; |
| environment: | ||
| sdk: ">=3.3.4 <4.0.0" | ||
| flutter: '3.38.9' | ||
| flutter: '3.41.6' |
There was a problem hiding this comment.
The PR description says it fixes #246 by adjusting the Write NFC Tags input (minLines/maxLines behavior), but this diff only updates the pinned Flutter version and localizes a few strings in image_editor.dart. As-is, it doesn't modify the NFC write UI (e.g. lib/ndef_screen/widgets/nfc_write_card.dart) where the reported alignment/line-wrapping issue lives, so it likely won't resolve #246. Either include the intended NFC UI changes in this PR or update the title/description to match what is actually being changed.
5d35691 to
2921089
Compare
2921089 to
e782793
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #246
Changes made:
Screenshots: