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
Empty states design updates #3879
Empty states design updates #3879
Conversation
- Changed body to "@style/TextAppearance.Woo.Body1" - Changed image padding to "@dimen/major_300"
You can test the changes on this Pull Request by downloading the APK here. |
Hey, @hafizrahman these are looking great however, some of the body copy looks a little small to me. Do you think you'd be able to double-check these, please? Also the pink used looks a little different (not sure if it's just the screenshots) we use Pink 50 for our primary buttons in light and Pink 30 when in dark mode. Cheers! |
<LinearLayout | ||
android:id="@+id/msg_wrapper" |
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.
I don't know if we want to wrap the images and text in a LinearLayout
. The idea of using a ConstraintLayout
is to ensure that there is a flat hierarchy. If you're looking to vertically align the image and text in the screen, you can change the value of layout_constraintVertical_bias=0.7
in the jetpack_required_msg
. That would achieve the same effect.
Before
After
@@ -51,7 +51,7 @@ | |||
<com.google.android.material.button.MaterialButton | |||
android:id="@+id/empty_view_button" | |||
style="@style/Woo.Button.Colored" | |||
android:layout_width="@dimen/alert_dialog_max_width" | |||
android:layout_width="@dimen/empty_state_button_width" |
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.
I don't think we want to set a fixed width to the button. If we want to increase the size of the button, we could always set the width to match_parent
and add margins.
I tried playing around with the button width and adding the following changes to the xml
file, gave me the same results. WDYT?
<com.google.android.material.button.MaterialButton
android:id="@+id/empty_view_button"
style="@style/Woo.Button.Colored"
android:layout_marginStart="@dimen/major_300"
android:layout_marginEnd="@dimen/major_300"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="0dp"
android:layout_marginBottom="@dimen/major_200"
android:text="@string/share_store_button" />
…2-empty-states-design-updates
Thank you for the reviews! Re: Button colors:The button currently uses
Or in other words, they're already set as Pink 50 in light mode and Pink 30 in dark mode 🤔 I tested this again in an emulator and the color looks correct: @anitaa1990 I wonder if you can help check the above and see what the color looks like in your device(s)? I'm curious if the problem is specific to my device or something else is going on. Re: Body font sizeThe body text in the empty view is currently set as
We're interested in the
So, the app is already being told to display the font as Roboto at 16sp, which matches the design document: I'm not sure where the differences come from. Perhaps it's again back to my specific device and the way I set global font size in it? @anitaa1990 Any ideas? |
Generated by 🚫 dangerJS |
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.
LGTM
This is to fix #3862
Here are the list of items being worked on in this PR. The items in parenthesis are the values used in the code.
Empty State Screen in Light Mode
@color/woo_gray_0
which translates to#F6F7F7
)@style/TextAppearance.Woo.Headline6
)@style/TextAppearance.Woo.Body1
)@dimen/empty_states_button_width
)"Not a WordPress site" screen:
@dimen/major_300
)@style/TextAppearance.Woo.Body1
)Site needs Jetpack screen:
"Print with your device" screen
Testing Instructions:
(please note that the "before" screenshots are from the
develop
version)Empty State Screen in Light Mode
"Not a WordPress site" screen
"Site needs Jetpack" screen
"Print with your device" screen in white background:
Update release notes:
[x] I have considered if this change warrants user-facing release notes and have added them to
RELEASE-NOTES.txt
if necessary.