Darkmode #61
Darkmode #61
Conversation
Pull request inspection and corrections
Revert "Pull request inspection and corrections"
Master to fork master
packages/smooth_app/lib/bottom_sheet_views/user_contribution_view.dart
Outdated
Show resolved
Hide resolved
|
@M123-dev maybe check if you pulled the lasted version from the Master branch, I changed the smooth_alert_dialog.dart filename to match the naming convention, and also corrected the SmoothAlertDialog class (as @monsieurtanuki noted the context attribute is no longer need). I haven't pull it yet to try but thanks for the PR I will check it out 🙂 |
|
I find the result quite stunning, well done @M123-dev I'm posting some pictures for those you want to see the result without having to build it : Screenshots
|
|
@PrimaelQuemerais Thank you for the kind words, my branch is up to date with the master, I don't know why the file has renamed itself again. |
|
@M123-dev that is weird because it doesn't match the file (filename and code) in the Master branch cf: smooth_alert_dialog.dart |
|
@monsieurtanuki No problem, I'm also not completely satisfied with the final product myself. Especially the limitations of |
|
@PrimaelQuemerais yeah I have seen it. It should be fixed now😁 |
|
Today I revised the PR again. Now all formatting errors should be gone and the navigation bar is now fully supported too. The widgets in the camera function are still normal, in my opinion they are only of little relevance, since most of the screen is covered with the camera image anyways. Of course this should still be added, but this would make more sense after the rebuild of the scanpage #62. (and when scanning works again) When you get on the profile page, there is an error, |
|
@M123-dev I noticed that the My temporary suggestions, just to locate the issue:
Tell me if it worked. |
|
@monsieurtanuki That is also what I suspected, however, each change has destroyed any properties of the toggle for me. The first works great, thanks for the tip |
packages/smooth_ui_library/lib/navigation/widgets/smooth_navigation_bar_classic.dart
Outdated
Show resolved
Hide resolved
packages/smooth_ui_library/lib/navigation/widgets/smooth_navigation_bar_classic.dart
Outdated
Show resolved
Hide resolved
| @@ -84,9 +91,10 @@ class _SmoothNavigationBarClassicState extends State<SmoothNavigationBarClassic> | |||
| .textTheme | |||
| .bodyText1 | |||
| .copyWith( | |||
There was a problem hiding this comment.
I assume that bodyText2 would be much better than bodyText1.copyWith(...).
I'm not an expert on themes, but I think the point is to have homogenous colors and text sizes.
Of course for the moment it's a work in progress for all of us, but it would make sense to say: "OK guys, now we use themes, here are the available colors and text sizes, and that's it, no improvisation for each new widget."
What do you think of that?
There was a problem hiding this comment.
When designing the themes, I built in bodyText2 as something like onPrimary, which should be used as the opposite of bodyText1 in terms of colors. (Such as the icon on the scan button)
bodyText2 currently has no use at all, that means we can use it for all kinds of purposes.
however, I think it is easier to use copyWith in this case, because it isn't a recurring text.
But yes, I agree with you we have to bring better structures into development. Such a kind of contribution guide that lists how to access the translations and how to add something to them, how to access the style elements and code guidelines.
Also, I think we need a better way of project planning, currently there are two roadmaps (README, Google Docs). Also, a big part of the documentation seems outdated.
@PrimaelQuemerais is currently working on a new one, when this is finished I'll look at it and attach a documentation of the data I have brought in and from what I is important.
But since this app is still in its early stage, we probably still have a bit time before we have to think about it seriously.
There was a problem hiding this comment.
The other suggestions you made are now in PR.
packages/smooth_app/lib/main.dart
Outdated
| Future<void> main() async { | ||
| await Sentry.init( |
There was a problem hiding this comment.
I guess this Sentry part should be part of a separate PR related to #9. This PR is about dark mode.
There was a problem hiding this comment.
I just thought that for the small change it is unnecessary to open a new pr
There was a problem hiding this comment.
I understand, but that's a source of confusion between issues, and there's no obvious sign that you fully tested the Sentry part. Perhaps other contributors that know things about Sentry and nothing about dark mode would like to make suggestions about Sentry, but it will be hidden "in the dark" ;)
That definitely makes sense to create a specific PR for Sentry, and you even already have a specific issue for that (#9).
|
From my side the pr would be ready now, does anyone still have concerns |
|
@M123-dev I'll review this PR within a couple of days. After this PR is merged, we'll talk about how we should use providers, shared preferences and themes in this project. |
There was a problem hiding this comment.
@M123-dev As I said, there are things that I would have coded differently, and there are topics we should address in general, like the target use of providers, shared preferences and themes.
But there's nothing I read that would prevent me from approving this PR now.
|
@monsieurtanuki Thanks again for the feedback and yes as I said, if there are fixed guidelines, I will restructure the code again. |
Like in Issue #22 requested here is a working darkmode, which can be switched on / off on the profile page. The setting is saved via shared prefs.
The darkmode works with the help of provider and does not need a rebuild. A large part of the widgets now support the darkmode function.
The logic is located in the themes folder and there is still only the standard theme in which the variations are specified.
The most important thing that is still missing is the text and the scanbutton and its shadow in the navbar, there I could not follow the code to manipulate the colors.
I am not a graphic designer and this is only the basic principle, everyone is welcome to override my color settings and equip the rest of the widgets with the dark mode.