fix: UI for the app bar and optimized filter selection logic#43
fix: UI for the app bar and optimized filter selection logic#43kienvo merged 6 commits intofossasia:mainfrom
Conversation
Reviewer's GuideRefactored the AppBar in ImageEditor to replace the default title/actions with a flexibleSpace-based layout featuring a centered title and two buttons, adjusted toolbar dimensions and styling, and updated the empty-state prompt text and style in the image list widget. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @Vishveshwara - I've reviewed your changes - here's some feedback:
- Consider extracting the repeated TextButton style into a shared widget or helper method to reduce duplication.
- Since your flexibleSpace only has a single centered Column, you can simplify the layout by dropping the Stack wrapper.
- The AppBar uses a hardcoded toolbarHeight of 100; consider using theme-based or responsive sizing to maintain consistency across devices.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| ), | ||
| const SizedBox(height: 6), | ||
| Row( | ||
| mainAxisAlignment: MainAxisAlignment.center, |
There was a problem hiding this comment.
suggestion: Duplicate TextButton styles
Extract the shared style into a variable or custom widget to avoid duplication.
Suggested implementation:
toolbarHeight: 100,
title: null,
actions: const [],
flexibleSpace: SafeArea(
child: Stack(
children: [
Center(
child: Column(
// Place this at the top of your widget/class/file, outside of build:
final ButtonStyle sharedTextButtonStyle = TextButton.styleFrom(
backgroundColor: Colors.white.withOpacity(0.2),
foregroundColor: Colors.white,
);
// ...then, wherever you use the TextButton:
padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
child: TextButton(
style: sharedTextButtonStyle,
- Replace all other duplicated
TextButton.styleFrom(...)usages in this file withsharedTextButtonStyle. - If the shared style needs to be accessed in multiple widgets, consider moving it to a separate file or making it a static member of a relevant class.
- Adjust the placement of the
sharedTextButtonStylevariable as needed to ensure it is in scope for all usages.
| "Please import an image to continue!", | ||
| style: TextStyle(fontSize: 16, fontWeight: FontWeight.w500), | ||
| "Please import an image or open the editor to continue!", | ||
| style: TextStyle(fontSize: 13, fontWeight: FontWeight.w400), |
There was a problem hiding this comment.
nitpick (performance): Make TextStyle const
Using const here allows for compile-time optimization since TextStyle is immutable.
|
@Dhruv1797 is this UI good? do you have any suggestions? |
|
Better use icons. |
|
Updated UI @kienvo @Dhruv1797 @AsCress updated.UI.mp4 |
|
Fixes #52 selection.logic.fix.mp4 |


Updated UI for the Appbar
Fixes #42
Fixes #52
Previous UI:

Updated UI

Summary by Sourcery
Revise the UI of the app bar in the image editor to use a flexibleSpace layout with centered title and action buttons, and update the empty-state prompt in the image list for clearer user guidance.
Enhancements: