feat: Enhanced UI and Functionality for the Select Screen #18

Vishveshwara · 2025-04-10T15:08:13Z

Fixes #17

Changes:

Added new DisplaySelectionScreen with grid layout for display selection
Created display cards with:
1. Display preview images
2. Color indicator dots
3. Technical specs (resolution, aspect ratio, driver)
Implemented filter chips for All/Color/B&W/HD displays
Created DisplayModel class with:
1. Display specifications
2. Color parsing
3. Aspect ratio calculation
4. PPI calculation
Created DisplayProvider for:
1. JSON data loading
2. Filter management
3. Selection state
4. EPD class mapping
Added displays.json with display configurations for easy support for more displays.

Screenshot:

Summary by Sourcery

Implement a comprehensive display selection screen with advanced filtering and display information features for the ePaper application

New Features:

Create a new display selection screen with a grid layout for display options
Implement dynamic filtering of displays by type (Color, B&W, HD)
Add display cards with detailed technical specifications and preview images

Enhancements:

Develop a flexible DisplayModel class to manage display metadata
Create a DisplayProvider for managing display data and selection state
Implement a reusable display card widget with comprehensive display information

Chores:

Add displays.json configuration file for easy display management
Refactor main application routing to use new display selection screen

sourcery-ai · 2025-04-10T15:08:18Z

Reviewer's Guide by Sourcery

This pull request introduces a new display selection screen with enhanced UI and functionality. It includes display cards with preview images and technical specifications, filter chips for display types, a DisplayProvider for data management, and a DisplayModel class to represent display specifications. The changes also involve adding a JSON file for display configurations and updating the main application to include the new screen.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change	Details	Files
Introduced a new screen for display selection with a grid layout.	Created a new DisplaySelectionScreen widget. Implemented a grid view to display available ePaper displays. Added a header and subtitle for screen context. Included a continue button to proceed to the image editor.	`lib/view/display_selection_screen.dart`
Implemented display cards to show display previews, color indicators, and technical specifications.	Created a DisplayCard widget to encapsulate display information. Added display name and size to the card. Included color indicator dots and color names. Displayed technical specifications such as model, resolution, aspect ratio, and driver. Added a border to indicate selection status.	`lib/view/widget/display_card.dart`
Added filter chips for display types (All, Color, B&W, HD).	Implemented filter chips to categorize displays. Created a FilterChipOption widget for selectable filter options. Managed active filter state using DisplayProvider. Updated display grid based on the selected filter.	`lib/view/display_selection_screen.dart` `lib/view/widget/filter_chip_option.dart`
Created a DisplayProvider to manage display data, filtering, and selection state.	Loaded display configurations from a JSON file. Parsed display specifications, including colors and resolution. Implemented filtering logic based on display type (All, Color, B&W, HD). Managed the selected display state. Mapped display configurations to EPD classes.	`lib/provider/display_provider.dart`
Defined a DisplayModel class to represent display specifications.	Included properties for display name, size, resolution, colors, and driver. Added computed properties for color display, HD status, PPI, and aspect ratio. Implemented color parsing and aspect ratio calculation.	`lib/model/display_model.dart`
Added a ColorDot widget to visually represent display colors.	Created a ColorDot widget to display a colored circle. Added support for indicating selection status with a shadow effect.	`lib/view/widget/color_dot.dart`
Added a JSON file to store display configurations.	Created a displays.json file to store display configurations. Included display properties such as name, size, resolution, colors, and driver.	`assets/displays.json`
Updated main.dart to include the DisplayProvider and DisplaySelectionScreen.	Added DisplayProvider to the MultiProvider. Set DisplaySelectionScreen as the home screen.	`lib/main.dart`
Added image assets for displays.	Added assets/images/displays/ to pubspec.yaml. Added assets/displays.json to pubspec.yaml.	`pubspec.yaml`

Assessment against linked issues

Issue	Objective	Addressed
#17	The app should support a wide range of displays from different vendors.	✅
#17	The app should accommodate various sizes, color configurations, and other essential specifications of displays.	✅
#17	Enhance the UI of the select display screen.	✅

Possibly linked issues

Enhance the Functionality and UI for the Select Display Screen #17: The PR implements the enhanced UI and functionality described in the issue, including grid layout, display cards, filter chips, data loading, and display configurations.

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!
Generate a plan of action for an issue: Comment @sourcery-ai plan on
an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @Vishveshwara - I've reviewed your changes - here's some feedback:

Overall Comments:

Consider using a consistent naming convention (e.g., camelCase) for model properties like ModelName.
The _getEpdForClassName method in DisplayProvider could be refactored to use a map for better readability and maintainability.

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

mariobehling · 2025-04-10T15:46:14Z

Thanks, please follow the colors same as in the badge magic app https://github.com/fossasia/badgemagic-app.

Use white as a background color for example.

Vishveshwara · 2025-04-10T18:34:03Z

Thanks, please follow the colors same as in the badge magic app https://github.com/fossasia/badgemagic-app.

Use white as a background color for example.

Thank you for your feedback, please review the updated colors below:

Updated UI

AsCress · 2025-04-16T18:08:41Z

@mariobehling We might want to review the logic used here. This doesn't align with the structure developed by @kienvo.
@kienvo Could you please see to this once ?
@Vishveshwara Also, please remove any LLM comments before you commit code. I also advise you to have a look at the logic and understand it first rather than relying on an LLM for the same.

Vishveshwara · 2025-04-17T18:23:00Z

@AsCress
I appreciate the call for careful review, but I have to admit it’s a bit disheartening to have my core logic attributed to an LLM. While I did use an AI tool to draft a few inline comments for improving readability and improving the names of variables, every function and data model implementation was carefully thought by me to be scalable such as the json display handling and filter chips.

AsCress

@Vishveshwara Understand the fact that using an LLM to generate code isn't bad. However, if you do so, the code has to be cleaned properly before you commit it. LLM code contains numerous things which are either irrelevant / not needed. The right approach is to always get an understanding of where we are, what functionality we want to achieve and then decide upon an approach for the same which takes into account the exact requirements and constraints of the project, something which an LLM has no idea about.

AsCress · 2025-04-18T17:17:57Z

assets/displays.json

@@ -0,0 +1,26 @@
+{


Why would be actually needing JSON storage for this ? The number of displays that we have to interact with are very limited and we already have an abstract class Epd, which is extended to model a display. Those classes could simply be modified to add whatever more parameters you want to show about the displays.

AsCress · 2025-04-18T17:19:18Z

assets/displays.json

+      "colors": ["black", "white", "red"],
+      "driver": "UC8253",
+      "imagePath": "assets/images/displays/epaper_3.7_bwr.png",
+      "epdClass": "Gdey037z03"


Also, how is this approach scalable, when we are anyways creating a new class for any new display that we want to add ?

AsCress · 2025-04-18T17:20:12Z

lib/constants.dart

+
+//Colors used in the app
+// Primary Colors
+const Color colorPrimary = Color(0xFFD32F2F); // You can adjust this color value


Please remove any such LLM comments before committing code.
If you do want to add comments to your code, add proper documentation comments for every function you add, so that it could also be used to generate documentation.

AsCress · 2025-04-18T17:26:37Z

lib/main.dart


 void main() {
  runApp(MultiProvider(providers: [
    ChangeNotifierProvider(create: (context) => ImageLoader()),
+    ChangeNotifierProvider(create: (context) => DisplayProvider()),


DisplayProvider() is something which we are using to manage and select displays only. There's no need to register it as a ChangeNotifierProvider() in main() for the whole app.

AsCress · 2025-04-18T17:28:49Z

lib/model/display_model.dart

+  }
+
+  // Helper method to find GCD for aspect ratio calculation
+  int _findGCD(int a, int b) {


Why a separate function for GCD? Core dart libraries can be used for this purpose.

AsCress · 2025-04-18T17:50:27Z

lib/model/display_model.dart

+import 'package:magic_epaper_app/util/epd/edp.dart';
+
+class DisplayModel {
+  final String id;


Having so many parameters for each display (id, name, ModelName) doesn't make sense to me. This just increases complexity.

AsCress · 2025-04-18T17:51:52Z

lib/provider/display_provider.dart

+  }
+
+  // Helper method to get the appropriate EPD based on class name
+  dynamic _getEpdForClassName(String className) {


Another function which just increases complexity, if you're aiming for a scalable approach.

AsCress · 2025-04-18T17:52:59Z

lib/provider/display_provider.dart

+      // Notify listeners that the data has changed
+      notifyListeners();
+    } catch (e) {
+      print('Error loading displays from JSON: $e');


Avoid having print() in your code. Use a logging library instead.

AsCress · 2025-04-18T17:55:58Z

lib/view/widget/display_card.dart

+class DisplayCard extends StatelessWidget {
+  final DisplayModel display;
+  final bool isSelected;
+  final VoidCallback onTap;


Why not just simply wrap DisplayCard in a GestureDetector, wherever used (in display_selection_screen.dart).

AsCress · 2025-04-18T17:58:02Z

lib/view/widget/color_dot.dart

+  const ColorDot({
+    super.key,
+    required this.color,
+    this.selected = false,


Do you use selected anywhere ? Should only add parameters whenever needed.

Vishveshwara · 2025-04-19T07:10:07Z

@Vishveshwara Understand the fact that using an LLM to generate code isn't bad. However, if you do so, the code has to be cleaned properly before you commit it. LLM code contains numerous things which are either irrelevant / not needed. The right approach is to always get an understanding of where we are, what functionality we want to achieve and then decide upon an approach for the same which takes into account the exact requirements and constraints of the project, something which an LLM has no idea about.

@AsCress
I apologize for the confusion. I did discuss the app’s requirements with @kienvo — namely, supporting multiple e‑ink displays with different drivers, resolutions, and colors(yellow). That’s why I initially separated the display selection UI into its own JSON‑driven configuration. However, I see now that it’s introducing unnecessary complexity at this stage. I’ll simplify the implementation, remove any extraneous LLM‑generated comments, and refocus on a cleaner, more maintainable approach that aligns with our project constraints. I really appreciate you a lot for helping me understand my mistakes, I’ll take this feedback seriously and work on these improvements with a positive, focused approach.

this is the reference app, which was mentioned in the ideas list, which supports multiple displays.

kienvo · 2025-04-19T10:32:26Z

@Vishveshwara, your proposed UI looks good. The thing is, the Epd class was there. You just have to improve it instead of relying on LLM. You could use it as a base, resolve the reviews, then we'll have a nicer app.

To resolve the reviews, add incremental commits to this branch, or commit with --amend and force push to this branch.

Enhanced UI and Functionality for the Select Screen

1d5db51

sourcery-ai bot reviewed Apr 10, 2025

View reviewed changes

Updated Colors for the select screen

2d829cd

mariobehling requested a review from kienvo April 10, 2025 20:20

mariobehling approved these changes Apr 16, 2025

View reviewed changes

mariobehling merged commit 926d55b into fossasia:main Apr 16, 2025
2 of 3 checks passed

AsCress reviewed Apr 18, 2025

View reviewed changes

Vishveshwara mentioned this pull request Apr 20, 2025

fix: changes the implementation of the display selection screen #23

Merged

AsCress mentioned this pull request Apr 22, 2025

feat: draw-canvas-feature-to-display-on-badge. #21

Closed

github-actions bot added a commit that referenced this pull request Jun 18, 2025

[Unsigned APK] v0.7.0 (#18)

db85835

feat: Enhanced UI and Functionality for the Select Screen #18

feat: Enhanced UI and Functionality for the Select Screen #18

Uh oh!

Conversation

Vishveshwara commented Apr 10, 2025 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide by Sourcery

File-Level Changes

Assessment against linked issues

Possibly linked issues

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

mariobehling commented Apr 10, 2025

Uh oh!

Vishveshwara commented Apr 10, 2025

Uh oh!

Uh oh!

AsCress commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vishveshwara commented Apr 17, 2025

Uh oh!

AsCress left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vishveshwara commented Apr 19, 2025

Uh oh!

kienvo commented Apr 19, 2025

Uh oh!

Uh oh!

Vishveshwara commented Apr 10, 2025 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Apr 10, 2025 •

edited

Loading

AsCress commented Apr 16, 2025 •

edited

Loading