Increased the font size used in widget buttons #5883
Conversation
|
@lognaturel how does the new font size look to you? |
|
Looks much more balanced to me! Thanks so much for doing another round. 🤗 |
| <item name="android:paddingTop">12dp</item> | ||
| <item name="android:paddingBottom">12dp</item> | ||
| <item name="android:textAppearance">?textAppearanceBodyMedium</item> | ||
| </style> | ||
|
|
||
| <style name="Widget.Collect.Button.Icon" parent="Widget.Material3.Button.Icon"> | ||
| <style name="Collect.Button.Icon" parent="Widget.Material3.Button.Icon"> |
There was a problem hiding this comment.
I know it ends up being weird because of our own "widget" term, but the idea of having Widget. as a prefix here is to differentiate between styles that are uses for views and themes (which use Theme.). You'll see the Material and AppCompat styles and themes follow the same pattern. Could we keep Widget. and do Widget.Collect.Button.QuestionWidget instead?
There was a problem hiding this comment.
But Widget indicates it's used by form elements (questions) and here it's not true because those buttons might be used in other places in the app.
There was a problem hiding this comment.
Right, but that's because we use the term "widget" to mean "question widgets" whereas Android uses it to just mean "view". You'll see all the Material3 (and pretty much any of the Android library) styles use Widget. as the prefix for view styles.
If it feels really confusing, maybe we should use Style. for view styles so they are still differentiated from themes (using Theme.)? We'd want to make that change everywhere though as there are other styles outside of buttons.xml that follow the Android naming convention. More radically, we could drop our prefix and just use style names like Button.Icon (like in the custom style examples here. If we do deviate from our current approach, we'll need to update CODE-GUIDLINES.md (under the "XML style guidelines" section).
There was a problem hiding this comment.
Ok, that makes sense, done.
| <item name="materialButtonOutlinedStyle">@style/Collect.Button.OutlinedButton</item> | ||
| <item name="borderlessButtonStyle">@style/Collect.Button.TextButton</item> | ||
| <item name="materialAlertDialogTheme">@style/Theme.Collect.Dialog.Alert</item> | ||
| <item name="widgetMaterialButtonStyle">@style/Widget.Collect.Button</item> |
There was a problem hiding this comment.
I really like the approach of using a theme attribute here. That would support the idea we've talked about of having a widgets module as we'd be able to declare and use the attribute there and have collect_app actually declare the styling.
Could we move this attribute lower in the file and add a comment that it's an attribute for question widgets? I think also naming it questionWidgetButtonStyle or widgetButtonStyle would make it clearer that it's not part of Material Components.
| @@ -6,5 +6,6 @@ | |||
| <attr name="materialButtonOutlinedIconStyle" format="reference" /> | |||
| <attr name="materialButtonIconStyle" format="reference" /> | |||
| <attr name="borderlessButtonIconStyle" format="reference" /> | |||
| <attr name="widgetMaterialButtonStyle" format="reference" /> | |||
There was a problem hiding this comment.
This should probably be in its own section as it's got nothing to do with Material Components.
|
Tested with Success! Verified on device with Android 13 Verified cases:
|
|
Tested with Success! Verified on device with Android 10 |
Why is this the best possible solution? Were any other approaches considered?
I have compared font styles used in different places the result is:
I think that 16sp would be a good solution... 16 is more than 14 (current size) and less than 18 (that was too much)
We can change
textAppearanceBodyMediumtotextAppearanceBodyLargeand it will increase font size from 14 to 16How 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?
Only the font size of widget buttons should be changed, nothing else. The navigation buttons (Back/Next), the buttons displayed in the save form dialog, etc should not be affected.
Do we need any specific form for testing your changes? If so, please attach one.
No.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
./gradlew connectedAndroidTest(or./gradlew testLab) and confirmed all checks still pass