Use Material 3 as base theme #5676
Conversation
seadowg
force-pushed
the
material-3
branch
2 times, most recently
from
96497f8
to
1040973
Compare
|
@alyblenkin this is probably in a good place to get some feedback. The buttons are pretty inconsistent, but that's something I think we should leave for #5567. It's good to check the Material 3 spec while looking through as some things surprised me (like how dialogs are now "primary" rather than surface coloured). There's probably a few things we want to adapt based on changes like that. |
|
@seadowg - sorry for the slow response on this! Yes, I'm seeing a lot of seeing surprises and inconsistencies! Some of the buttons didn't change and the dialogs have a light blue background. I might have done something weird - let me know if you want to chat Monday morning :)
|
Let's do it. Will send you an invite. |
|
@alyblenkin ok I think this is good for another check through! |
|
@seadowg, all the changes we spoke about last week look so much better! I did notice one small thing...it looks like the menu buttons are slightly darker. I colour-dropped it in Figma, and it seems to have a purple tint (#F1F0F5), but it's likely off because it's a screenshot but still darker. |
Good spot! I think mixed some colors up while moving them around. Have you also had a chance to review the dark theme? It feels like the changes are bigger there in terms of how the colors are applied. |
|
Yes, I forgot to say that the dark mode looks pretty great for the most part! I think the biggest issue I came across was the delete button being difficult to see, but I wasn't sure if that would be addressed in this issue or in #5567.
And I doubt we can change this, but it isn't super clear that this is a dialog. If we could make the background lighter, so the dialog pushes to the foreground that would be great.
|
I've made it a default Material 3 "Outlined Button" rather customising it to use our error color.
Ah good point. I've made it so that we use the default primary tinted dialogs in dark (but still surface colors in light). This looks the best to me. |
|
The delete button and dialog look great! I'm not sure if those changes impacted the main menu, but it's a little light 😄 |
Are you seeing that consistently? If so, what Android version are you on? It can't reproduce that. Sometimes things like this happen immediately after switching from Light to Dark (or the other way around). The way all that's implemented under the hood in Android is surprisingly hacky. |
|
Oh yes, playing around with the settings made it go away! |
|
There's a lot of annoying inconsistencies now with the buttons across the app, so I think I'll keep this PR open and do #5567 as part of it as well so we don't have to test/merge it in a confusing state. |
|
@alyblenkin this is ready for another look with hopefully unified buttons! One thing to note is that I've kept a "bar" behind all the bottom buttons (Go to start/Go to end, Back/Next etc) which differs from your mockups where a lot of these buttons float over the content. The reason for this is that otherwise, well need to do some work to add dynamic margins to the content so that enumerators are always to scroll to the bottom without having the buttons cover the very last chunk. This is all very possible (and you'll have seen things like this in loads of apps), I just wanted to focus on the button styles themselves for this, and we can come back for that "icing on the (dairy free) cake" another time. |
|
I did not mean to mark this pull request as ready, but I can't undo it! |
|
Fixed! |
|
Hello! I think it's easier to view the comments in figma. Let me know if you want to hop on a call to discuss |
|
@alyblenkin that's all the button styles updated with the increased ( |
|
The buttons are looking great One small thing, it looks like these buttons were super-sized |
Looks like our dynamic text sizing for widget buttons doesn't play well with that new padding. I'm going to make it so widget buttons use the normal button text size was their default (but still let the user bump it up or down). |
|
Fixed! |
|
@lognaturel the only controversial change here is 2646e4a. There I remove the ability to access "Get Blank Form" from Google Drive projects. My thinking is that this is ok because we are removing the ability to start blank forms anyway (in #5402). At the moment, clicking "Get Blank Form" does nothing. I'm thinking we should either hide the button, or pop up a dialog to explain what's going on (like in #5675). What do you think? |
We discussed on a call and decided to go with the latter. |
Closes #5607
Closes #5567
Blocked by #5675As well as the referenced issues, this removes the ability to download blank forms from Google Drive. This was done to avoid having to rework styles on the Google Drive screens that were not useful anymore as you're no longer able to fill out new forms in the next release.
What has been done to verify that this works as intended?
Existing tests and verified manually.
Why is this the best possible solution? Were any other approaches considered?
Not a lot to discuss here! I've been back and forth with @alyblenkin on how we implement Material 3 and we're feeling like we're a good place for now.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
I think a general check over the app would be good here. It definitely be good to hear about parts of the app that don't look right to anyone.
As I mentioned above, there have been some changes to Google Drive projects (you're no longer able to download forms) so that should be checked as well. The best way to do that is to ask @lognaturel to put a signed build together so that it's possible to use an old release to create the project and then upgrade.
Before submitting this PR, please make sure you have:
./gradlew checkAlland confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTestlocally.