-
-
Notifications
You must be signed in to change notification settings - Fork 286
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
fix: Personalize ranking top bar color #1238
fix: Personalize ranking top bar color #1238
Conversation
hi, |
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.
The code looks good @PrabeshPP, just two small comments.
Also the automated tests are failing because of formatting, Please run flutter format .
to automatically format the code
CHANGELOG.md
Outdated
## [0.2.0](https://github.com/openfoodfacts/smooth-app/compare/v0.1.0...v0.2.0) (2022-03-16) | ||
|
||
|
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.
Where do all these changes come from?
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.
These Changes came when I did pull request. However , on VS Code it only showed changes in personalized_ranking_page.dart .
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.
You probably created your branch from the release-please--branches--develop
and not from develop
itself. Don't worry I just reverted the changes in the changelog.
child: Text( | ||
widget.title, | ||
overflow: TextOverflow.fade, | ||
)), |
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.
Another ,
between those two would be good
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.
hi @M123-dev I have added , .
@@ -111,7 +113,7 @@ class _PersonalizedRankingPageState extends State<PersonalizedRankingPage> { | |||
(final int index) => Tab( | |||
child: Text( | |||
titles[index], | |||
style: TextStyle(color: colors[index]), | |||
style: index==0?TextStyle(color:themeData.hintColor ):TextStyle(color: colors[index]), |
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.
Could you reformat your code please?
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.
hi @g123k
I have now re-format the code.
@@ -54,6 +54,8 @@ class _PersonalizedRankingPageState extends State<PersonalizedRankingPage> { | |||
context.watch<ProductPreferences>(); | |||
final LocalDatabase localDatabase = context.watch<LocalDatabase>(); | |||
final DaoProductList daoProductList = DaoProductList(localDatabase); | |||
final ThemeData themeData = Theme.of(context); | |||
final ColorScheme colorScheme = Theme.of(context).colorScheme; |
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.
Since you already have the value of Theme.of(context)
one line beforr, please re-use it instead of requesting it once again
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.
hi @g123k thank you for the comment.Now,I have refactor the code.
LGTM 👍 |
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.
.
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.
Thanks @PrabeshPP everything looks good now
Congrats on your first PR here 🎉 |
What
Added dark theme for the top bar in Personalized Page.
Screenshot
Dark Theme:
LightTheme:
Fixes bug(s)
Part of
(please be as granular as possible)