-
Notifications
You must be signed in to change notification settings - Fork 0
refactor/ui-enhancements #130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add date as a separate text element above metadata chips - Convert metadata chips to horizontal scrolling ActionChips - Update chip design with reduced opacity background and increased font weight - Remove unnecessary padding and border settings from chips
- Remove InterstitialAdManager and related ad code - Extract headline tap handling logic to a separate class - Simplify item spacing in the feed - Update imports and remove unused code
…height - Replace SliverToBoxAdapter with SliverPadding for better alignment - Reduce the height of the metadata chips container from 52 to 36 - Adjust padding and clip behavior for improved visual appearance
…source_row - Change the export statement in feed_core.dart - This ensures the correct file (headline_source_row.dart) is being exported
- Create a new widget to display the source and publish date of a headline - Implement tap functionality to navigate to source details - Use timeago package to format publication date - Apply theming and styling according to existing design guidelines
- Add HeadlineSourceRow widget to HeadlineTileImageStart - Remove HeadlineMetadataRow from HeadlineTileImageStart - Adjust layout to accommodate new source row
- Add HeadlineSourceRow above the image - Remove HeadlineMetadataRow - Adjust padding and layout of title text - Improve image loading and error handling
- Remove HeadlineMetadataRow - Add HeadlineSourceRow - Adjust spacing between elements
- Change date format from 'MMM d, yyyy' to 'yyyy/MM/dd' for better internationalization support
- Add AppBloc dependency to get the current locale - Format relative dates using the current locale - Simplify some widget code by removing unnecessary braces
Summary of ChangesHello @fulleni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on significant user interface enhancements and code refactoring within the application's news display features. It streamlines how headline metadata, such as source and date, is presented across different views and improves the interactive elements like metadata chips. Additionally, it centralizes the handling of user interactions with headlines, particularly concerning ad integration, leading to a more consistent and maintainable architecture. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a series of valuable UI enhancements and refactorings. Key improvements include the creation of a standardized HeadlineSourceRow
widget, centralizing headline tap logic with HeadlineTapHandler
, and refining the layout on the headline details and feed pages. These changes enhance code maintainability and UI consistency. I've identified one critical layout bug that could cause a render overflow and a couple of medium-severity issues related to code consistency and maintainability. Overall, this is a solid step forward for the codebase.
visualDensity: VisualDensity.compact, | ||
materialTapTargetSize: MaterialTapTargetSize.shrinkWrap, | ||
), | ||
const chipAvatarSize = 18.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a hardcoded value 18.0
for chipAvatarSize
makes the code harder to maintain and less consistent with the app's design system. The previous implementation used AppSpacing.md
.
If 18.0
is a new standard size, consider adding it as a constant to your AppSpacing
class (e.g., AppSpacing.mdLg
). If an existing constant from AppSpacing
is suitable, please use that instead to maintain consistency.
Status
READY
Description
This pull request focuses on significant user interface enhancements and code refactoring within the application's news display features. It streamlines how headline metadata, such as source and date, is presented across different views and improves the interactive elements like metadata chips. Additionally, it centralizes the handling of user interactions with headlines, particularly concerning ad integration, leading to a more consistent and maintainable architecture.
Type of Change