-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add a Material3 based theme proposal #215
Conversation
Keeps M2 looking close to as before, but M3 looks more like M2
NOTE: Print, equality and hasCode are still incorrect
CHANGE: Themed Card style.
…the drawer also on other top level screens on phones.
…the drawer also on other top level screens on phones.
…the drawer also on other top level screens on phones.
Also automatic sort package import.
Card elevation remove, use themed values.
Other changes just Dart auto format.
The rest are are just dart format on auto save.
Yes.. this looks like a massive PR. Did you want me to accept everything and talk later or talk first and then merge. I looked at nearly all the individual comments. I think there will not be any problems.. but I'm not sure if it was complete fixes for all or if I use that and fix other things to match. I'm not so great at git.. so my other programmer told me to clone your stuff and test which is easier. My email is bksubhuti and I use gmail |
I tested both android and linux desktop.. The slider m3 bug assert disappeared. (sadhu x 3 as we monks say).. We have a prevention to stop the app from closing on the back button. The context lint across async gaps.. not sure how to fix that. |
I did notice that the search filter.. (you have to search for a word.. "try mettā") Then there is a half shown action button for filter.. these results are changed from last build. The filter button is half shown, the top "x" button is half shown .. and there are two buttons at the bottoms (select all and deselect all) which are barely shown. Other than that.. it looks like the transformation is complete. I think that "safe area" as you said will fix that. The tan reading mode is now an eyesore and needs to change to that light color theme color used in appbar. or a grey. I'll have to change it. I'll also have to remove the hard-coded blue from the chips. But I think we will keep the blue for pull handle to hide or show the bottom tool bar while reading.. The app is working well though on my phone and i'm using as my daily driver. We have many new features coming for our November release, but your PR will be the one feature people will notice and love the most. |
I'll have a look at that soon. Not at keyboard right now. Like I said, since I am not familiar with the app there were probably a few place I was not aware of to look at. Also the rotation bug should be fixable by just not selecting an index on the bottom nav if it is higher than 4, but it might mess with your navigation stack a bit, I can look at it too. I think your navigation stack might have some issues, I saw a bunch of animation asserts that looked related to it. Yes I also saw the back button prevention handler. That one is always good to have. But of course back should not appear when moving around on top destinations, it did sometimes, I removed it, but that it appeared also indicates there might be a top level navigation issue. Usually in phone mode the Drawer is shown on all top destination screens, not only Home like it is now. Of course on the Dictionary you have the popup guide there now, so there it cannot be added right now unless it is moved. Never got the app to build on Mac desktop, so I only tested on landscape Android phone and tablet emulator. Happy to help, I'll get back to you soon with more comments. Even if you merged it already (wow and cool 😎🙏), we could still do a Google video meet anyway just, would be nice to meet you. Plus if you have any questions or would just like to have some other look on the design anywhere discussed, it can certainly be done. I just made some initial quick minor design changes to allow the app to work well in both real M2 mode and M3 mode. |
Yes.. would be nice to talk with you. My first exposure to you was watching your interview video on flex themes. You can hear my programming story and why I ordained here . I was an msvc++ programmer in the 90's specializing in DB apps and hardware stuff. I did well and looked really good because nobody else knew the stuff back then. Now it is very common. I just pushed some padding to fix the search filter button and the chips. I added padding on it and it worked. I pushed those changes and also set the "medium page color" to the light version of the primary. It looks really nice. Seypia is dead.. looks like an eyesore compared to the beautiful stuff we have now. (btw: even though i take finals.. I'm old.. 53). The app is really nice looking .. we are so grateful for your help! I'm really weak in git.. so I just push directly as the changes come. Sometimes, I'll push broken code for the Bulgarian programmer to fix. I'd also like to know what happened to fix the debug assert when you press the slider and go back to settings. . (that is working fine now) I'll fix the ui for the themes settings to be an expansion tile in the next few days and move the page color there too. I sent a message to Tharindu who is the mac builder and submitter to find out why it does not build. Maybe there are some swift things he does. |
Tharindu said "Mainly it must be the application ID to something of his development account. But based on the error." |
I think xcode is needed. The ven pndazza said it worked fine but he
refused the upgrade. Perhaps you are using something different. Ven
pndazza does not have our app store certificates (neither do I), so it is
not dependent on it.
I did push the env file but if you are running Android, you have that
settled.
…On Tue, Oct 17, 2023, 16:53 Rydmike ***@***.***> wrote:
I'll have a look at that soon. Not at keyboard right now. Like I said,
since I am not familiar with the app there were probably a few place I was
not aware of to look at.
Also the rotation bug should be fixable by just not selecting an index on
the bottom nav if it is higher than 4, but it might mess with your
navigation stack a bit, I can look at it too. I think your navigation stack
might have some issues, I saw a bunch of animation asserts that looked
related to it.
Yes I also saw the back button prevention handler. That one is always good
to have. But of course back should not appear when moving around on top
destinations, it did sometimes, I removed it, but that it appeared also
indicates there might be a top level navigation issue.
For accidental back swipes it is very nice to have though. Matter of taste
if you want it on Home, some users like it some not. What I have done in
some apps make back go to home, if not on Home instead of exit, and then on
home exit, via confirmation and make a config whether it will exit via
confirmation on back from Home, so users can configure if you can just keep
back swiping to exit the app, or if it will ask for confirmation, when you
do so from top Home route. Most apps just silently exit when you do that,
so many users expect that.aa default behavior.
Usually in phone mode the Drawer is shown on all top destination screens,
not only Home like it is now. Of course on the Dictionary you have the
popup guide there now, so there it cannot be added right now unless it is
moved.
Never got the app to build on Mac desktop, so I only tested on landscape
Android phone and tablet emulator.
Happy to help, I'll get back to you soon with more comments.
Even if you merged it already (wow and cool 😎🙏), we could still do a
Google video meet anyway just, would be nice to meet you. Plus if you have
any questions or would just like to have some other look on the design
anywhere discussed, it can certainly be done. I just made some initial
quick minor design changes to allow the app to work well in both real M2
mode and M3 mode.
—
Reply to this email directly, view it on GitHub
<#215 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEKXQWHOYKRLOC4IWD4PLK3X7ZTBVAVCNFSM6AAAAAA6DEHWQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONRWGIYTEOJSHA>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
@bksubhuti, Just as info, I did manage to get the build working on macOS as well, as you suggested deleting the old macos folder and recreating it worked. I don't know if you want me to commit it or not. I did restore its asset icons from the deleted version with git, the rest is pretty much the newest macos folder created with Flutter 3.13.7. I also looked at the filter padding issues and it looks like you indeed figured that one out. I have been seeing the problem, saw it already yesterday, where the books disappear. They are there and then suddenly gone. like above on right. They were all then when I started the app, after creating the DB index etc, but are no after going e.g. to Settings. They won't come back unless I delete it it create again, but they always go away after navigating around a bit in the app. When that happens, all I get is the spinner: Have you seen that? |
Regarding more issues and any more contributions I do. I will post them as individual smaller issues and tasks in the repo issues first and then make PRs towards them. I can't make any major promises, but sure I can help occasionally, sounds like fun 😄 I guess I would start with first creating an issue with a overview of topics I suggest I can contribute to, and expand on each one of them with their own issue. Then when I (or anybody) does a PR to fix one, we can reference the issue and close it with a PR. The massive PR I did was not really good form, but there was at least one commit per file, so that made it a bit easier to see what was happening. Still one issue per PR is the way it should be. I would probably among those issues add some design proposals with images that explain my suggestions first. Mostly minor things. Ultimately it is all up to you what you think about them and how you want it to look. Anyway I'll throw in a list first of some topics I've noticed while digging around in the code as well as trying the app. Overall I'm very impressed by how far this app has come since I first saw it 👍🏻 |
I'm very happy to hear that you can dabble here and there on our project. It is way more advanced than my level. But I sort of know how things work to suggest fixes that I'm not technically capable to perform. I'm mostly widget and package placement programmer with some text and db stuff. I guess that can do a lot. It is sort of like sewing. It is easy .. but as long as the machine works and is tuned correctly. It is always bound to break though. I know this is bad form, but I'd rather just add you to the project as a contributor. I'm very weak in git, and I need a friend or chatgpt to bail me out when i need to do command line stuff. I trust you to make the changes, but I don't understand this level.
About the books disappearing .. this is a critical bug that needs to be reliably duplicated. I first saw it only once on my phone recently. The db is somehow corrupted and I'm not sure what is going on. It is concerning. I think it happened when I was testing your issue of trying to make the app crash by going to settings and rotating my phone. (it survived but the books disappeared). About the MacOS Build.. I'm glad you got it working with something simple. I will need to rename the git and project directory this November so this process is better. I also just randomly picked a url for our app rather than ensuring I owned it. I don't want to change it, but I will eventually need to. Let me see what flutter version the other macDevs are on. (com.paauk) After that, it would be good to push those files. (The vnpndazza is quite sick so it won't make much difference., and Tharindu, who builds apple stuff for us usually upgrades when I ask). I'll inform him. The buttons for the filter are fixed (plus I removed the blue color to default) but they are too much padded on desktop as a result.. So, I will put a conditional padding on based on desktop-tablet-view or mobileView. I also fixed the active tab color to match when in the middle-color-page-view (between dark and light). I wonder why m3 changes the layout position.. but I have and can manually tweak it. One change I'd like to see. The rest is really nice. I really enjoy simply looking at the app. |
I checked with Tharindu and he is fine with upgrading to 3.13.x . Our project is on his personal laptop. We would not be having these conversations if you were not trustworthy and monk-friendly. You seem to be "anybody-friendly" |
About the drawer as a widget. I sort of agree with that. We wanted the dictionary and settings to be available all of the time. You can see that we have dictionary available on the root menu. The dictionary can also be accessed on the pali reader as well by clicking on any word. I'd also like to discuss a strategy for removing the context across async. With localization and provider, we get stuck passing context. But perhaps that can be reworked in restructured.. We have a lot of providers running and I think it is getting to be a problem which is another issue. Ven pndazza said he'd use other provider models for his other projects. (he has many projects now). Localization and Themes were one of my first "tests" and assignments for us to join forces to make TPR. His original app was only Myanmar Language and script. He converted to roman while i did the localization and themes research. That was when you and I first met. I did it with the app Buddhist Sun which was mostly a learning project (proof of concept) for me. That is why I have a sqlite db on it. |
I finished my exams today if you wanted to talk. I linted about 100 items. Mostly super.key stuff. But got some of those out of the way.. Sometimes it complains about variables that are not used, but they really are used.. But it would be good to talk to you soon. We have a meeting at 7:30 pm and then I'm mostly free for most days and nights in the next week. We also hope to make a release very soon as well. |
I found this article.. on global keys to help prevent the linting error (besides excluding it :) ) |
Theme proposal and various fixes
Use or abuse the code contributions as you like.
If you want we can do e.g. a video call (if we find a suitable shared time slot) and discuss some of the theme styles proposed here and other changes too. There are so many possibilities and at the end of the day it is just preferences 😄
The PR contains a commit for each file, so it is easier to see changes per file and a brief comment per file too.
Material 3 theme mode
This PR adds a M3 based theme proposal and style, with color tints on Cards.
It keeps the old M2 based style available, but modernizes it with FlexColorScheme M3 look alike shapes, but keeps the M2 style AppBar and no color tints.
The style included some Card elevation and color fixes on them, that changes the style a bit.
The TabBar for the reader is now in AppBar when there is AppBar and otherwise outside it.
Android - Annotated Region - SystemUiOverlayStyle
An attempt to style the system navigation for Android builds was also added. It required some additional configs with SafeArea to make it looks nice. The AnnotatedRegion and SystemUiOverlayStyle settings only impact Android (phone and tablet), but the SafeAreas might needs double checking on all platforms. I might have missed some layouts I did not know about.
Usage of DropdownButton
Consider replacing all usage of DropdownButton, like e.g. lang selection on Settings, with newer widget that supports theming and M3 styling.
The bottom bar on the reader
The custom bottom bar on the reader that can slide away, can potentially also be styled a bit more using the active theme's ColorScheme, to make it fit better with the M3 style, when M3 mode is used. I can look at it later if it is of interest.
Lint and Analyzer Issues
I also fixed a large number of analyzer and lint issues, mostly minor nits.
There are however still 33 warnings and 106 hints that it complains about.
There are a lot of lint warning about not "Don't use BuildContext across async gaps". The lint is not always bad, but it can cause crashes in some cases. Some can be solved with unawaited(), while other might require StatefulWidget and checking if context is still mounted.
The "Invalid use of a private type in a public API" are generate when using old format of setting up StatefulWidget.
KNOWN/SEEN BUGS
There are bugs in the navigation. If you use phone and rotate it to landscape and select Setting from rail on the phone in landscape and then rotate phone to portrait the app will crash because it is index 5 (6th one), which does not exist as a selection when rotated back to the bottom NavigationBar.
Other navigation issues seems to be related to used navigation context and animation controller related to animation being disposed. This needs to be studied further.