feat: added side navigation bar #69
Merged
Conversation
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
Reviewer's GuideThis PR centralizes app navigation by introducing a CommonScaffold and AppDrawer for a consistent side navigation experience, refactors the existing display selection screen to adopt this layout, adds new About Us and Settings screens with enforced portrait orientation, configures named routes in main.dart, integrates URL launching and sharing via share_plus, and updates platform manifests and dependencies to support external links and new assets. Class diagram for CommonScaffold and AppDrawer integrationclassDiagram
class CommonScaffold {
+String title
+Widget? titleWidget
+Widget body
+int index
+List<Widget>? actions
+double? toolbarHeight
}
class AppDrawer {
+int selectedIndex
}
CommonScaffold o-- AppDrawer : uses
Class diagram for AboutUsScreen and SettingsScreenclassDiagram
class AboutUsScreen {
+State createState()
}
class _AboutUsScreenState {
+void initState()
+void _setOrientation()
+Widget build(BuildContext context)
}
AboutUsScreen -- _AboutUsScreenState
class SettingsScreen {
+State createState()
}
class SettingsScreenState {
+String selectedLanguage
+List<String> languages
+void initState()
+void _setOrientation()
+Widget build(BuildContext context)
}
SettingsScreen -- SettingsScreenState
Class diagram for URL launcher helperclassDiagram
class url_constant {
+Future<void> openUrl(String url)
}
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}
There was a problem hiding this comment.
Hey @Vishveshwara - I've reviewed your changes - here's some feedback:
- You might want to centralize the orientation-locking logic (currently duplicated in each screen’s initState) into a common mixin or widget to reduce boilerplate.
- The drawer’s navigation uses Navigator.pushNamedAndRemoveUntil which clears the entire stack—consider pushReplacementNamed (or similar) if you want to preserve expected back-stack behavior.
- There are several commented-out ListTile entries in the navigation drawer—removing or moving them to a feature branch will keep the code cleaner and more maintainable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You might want to centralize the orientation-locking logic (currently duplicated in each screen’s initState) into a common mixin or widget to reduce boilerplate.
- The drawer’s navigation uses Navigator.pushNamedAndRemoveUntil which clears the entire stack—consider pushReplacementNamed (or similar) if you want to preserve expected back-stack behavior.
- There are several commented-out ListTile entries in the navigation drawer—removing or moving them to a feature branch will keep the code cleaner and more maintainable.
## Individual Comments
### Comment 1
<location> `lib/view/widget/navigation_drawer.dart:166` </location>
<code_context>
+
+ Navigator.pop(context);
+
+ if (externalLink != null) {
+ openUrl(externalLink);
+ } else if (shareText != null) {
+ Share.share(shareText);
+ } else {
</code_context>
<issue_to_address>
External links are opened without checking if the URL is valid or can be launched.
Handle potential exceptions from openUrl to prevent unhandled errors, and consider notifying the user if the URL cannot be opened.
Suggested implementation:
```
if (externalLink != null) {
_handleExternalLink(context, externalLink);
} else if (shareText != null) {
Share.share(shareText);
} else {
```
```
}
Future<void> _handleExternalLink(BuildContext context, String url) async {
try {
final uri = Uri.tryParse(url);
if (uri != null && await canLaunchUrl(uri)) {
await launchUrl(uri);
} else {
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(content: Text('Could not open the link.')),
);
}
} catch (e) {
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(content: Text('An error occurred while opening the link.')),
);
}
}
```
```
import 'package:flutter/material.dart';
import 'package:url_launcher/url_launcher.dart';
import 'package:share_plus/share_plus.dart';
```
</issue_to_address>
### Comment 2
<location> `lib/constants/url_constant.dart:5` </location>
<code_context>
+
+Future<void> openUrl(String url) async {
+ final Uri uri = Uri.parse(url);
+ if (!await canLaunchUrl(uri)) {
+ throw 'Could not launch $url';
+ } else {
+ await launchUrl(uri);
</code_context>
<issue_to_address>
Throwing a string as an exception is not idiomatic in Dart.
Use Exception('Could not launch $url') instead for clearer error handling and stack traces.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
if (!await canLaunchUrl(uri)) {
throw 'Could not launch $url';
} else {
await launchUrl(uri);
}
=======
if (!await canLaunchUrl(uri)) {
throw Exception('Could not launch $url');
} else {
await launchUrl(uri);
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `lib/view/about_us_screen.dart:66` </location>
<code_context>
+ ),
+ const SizedBox(height: 30),
+ Text(
+ "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.",
+ textAlign: TextAlign.justify,
+ style: GoogleFonts.sora(
</code_context>
<issue_to_address>
Text content is concatenated without spaces, which may result in missing spaces between sentences.
Add a space at the end of each string segment to ensure proper spacing when the text is rendered.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@AsCress @kienvo @Dhruv1797 can you please review this PR |
Exception handling for the url Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
nope3472
reviewed
Jun 25, 2025
There was a problem hiding this comment.
1.Handle External Link Exceptions: External links are opened without checking if the URL is valid or can be launched.
2. Centralize Orientation-Locking Logic: The orientation-locking code is duplicated in each screen’s initState. Move this logic into a common mixin or widget to reduce repetition.
|
@Dhruv1797 can you please review this |
Dhruv1797
approved these changes
Jun 26, 2025
AsCress
reviewed
Jun 26, 2025
There was a problem hiding this comment.
@Vishveshwara Too much commented out code. Do we need that to be present in the repo ?
I have just commented on some TODO actions, like adding the link of the app from the app Store. |
AsCress
approved these changes
Jun 26, 2025
AsCress
changed the title
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.
Fixes #65
Screen Recording:
side.nav.bar.mp4
Summary by Sourcery
Add side navigation bar across the app by introducing a reusable CommonScaffold and AppDrawer, wire up named routes for display selection, settings, and about-us screens, and configure platform settings and dependencies to enable external URL launching and sharing.
New Features:
Enhancements:
Build: