Open osullivanchris opened 3 years ago
@ashiagr - Researched a bunch of these today, let's sync tomorrow and see what direction we should take on the first one. @osullivanchris - hopefully I have provided some food for thought. Thanks for pulling this together for us.
Text Styes
The snackbar shown is the default snackbar for the app. It's material design and is styled based on our app theme (light/dark). It's not impossible to change, but definitely not a low hanging fruit type of fix. We have a couple of options here.
(1) Leave it as is to match the rest of the app
(2) Some form of WPDialogSnackbar
. The existing layout is different than what we are now showing post migration.
(3) Set the text color dynamically. We could do something like this: New data added to SnackbarMessage
for text color, which is then passed into SnackbarItem
. Then in SnackbarSequencer.prepareSnackbar
apply the needed parts like so:
snackbar.setTextColor(resourceProvider.getColor(android.R.color.``white``))
We’d also need to intentionally take care of dark vs. light programmatically.
Dead pixels above Snackbar
This is definitely preexisting because I can see it on Reader snackbars too - imo it's the shadow. I’m not sure what, or if, we should be doing in this instance. For reference the styles are
WordPress.Snackbar
which inherits from Widget.MaterialComponents.Snackbar
which inherits from Widget.Design.Snackbar
. Making one change has repercussions all over. 😞
Area hidden after Snackbar dismisses Yes, I see this happening. The space around the snackbar isn’t released. Feels like something funky with the coordinator layout. I can reproduce this over and over. I'll need to research this more.
Snackbars disappear quite quickly
Previously, did it auto-dismiss? I think you said it did, but maybe had a longer duration? @ashiagr - can you jump in to on the auto-dismiss question. I see a setting of 7 seconds for the snackbar, but I wonder if we are getting into a sequencer dismiss, show next. Will need to dig in deeper on this one.
End of tour
There's no success indication in the My Site UI. Was this the case before
Yes, this was the case before.
The Quick Start card disappears immediately. Was this the same as before? I'd be tempted to show it completed after the last step, then dismiss it after 1 day or similar afterwards.
This is the expected behavior - old and new. Any changes to this behavior would be outside the scope of Phase 1.
@zwarm thanks for the info!!
Text Styles
Here are two examples of Snackbars that we already have which use a mix of grey text, and white text for emphasis. As these had not changed, I thought they already were using the native component. And if so, we could do the same thing in the other ones.
If my assumption is incorrect and these are also custom, then we're not getting something for free like I thought?
Dead pixels above Snackbar
Yeah I have seen it before too. So I figured it could be pre-existing. Wish we could have it fixed. Maybe not for this project?
Snackbars disappear quite quickly
Let me know if you find out more. @ashiagr mentioned something about the time decreasing, I think. Maybe its a case of just increasing the time a little. Or maybe more complex with the sequence as you mentioned.
End of tour: no success indications, disappears immediately
Understood its out of scope. But if we were to address it in another phase, do you think it would be better to show in a completed state? Or any other opinion? I think it would be good if it showed as complete, and hung around for a little while. But might just be me.
Delayed Snackbars
This is due to how snackbar sequencing works at the moment. It adds a delay = snackbar duration just after the snackbar is shown so that the next snackbar does not overlap with the current one and users get enough time to understand the messages. See https://github.com/wordpress-mobile/WordPress-Android/pull/10856#issuecomment-604452148 for more details. We did not use the snackbar sequencer in the old implementation.
We can fix this by not adding this delay for quick start snackbars with action buttons, where we immediately want to show the next task snackbar. I'll shortly submit a PR.
Snackbars disappear quite quickly
There are two kinds of quick start snackbars:
Here are two examples of Snackbars that we already have which use a mix of grey text, and white text for emphasis. As these had not changed, I thought they already were using the native component. And if so, we could do the same thing in the other ones. If my assumption is incorrect and these are also custom, then we're not getting something for free like I thought?
@osullivanchris those snackbars are not custom, they too are the default for the app (themed, etc). The textColor is not white
but colorOnSurface
(to account for light/dark), the icon itself is a white drawable. The other text areas that look white are jsut wrapped in a <b></b>
block. The only change we made to snackbar was to set the maxLines=5 - everything else remains the same as before.
The old quick start notices were custom (see the screenshot below). We changed these from custom to default.
@osullivanchris
Understood its out of scope. But if we were to address it in another phase, do you think it would be better to show in a completed state? Or any other opinion? I think it would be good if it showed as complete, and hung around for a little while. But might just be me.
I could easily see this as a congratulations bottom sheet (like the post reminders) popup. Setting a local reminder to remove the card would be more involved, but IMO that particular card takes up a fair amount of ATF space and should be removed when complete.
@osullivanchris @ashiagr Area hidden after Snackbar dismisses After some more research, this is also a pre-existing condition. :sigh . So not going to address those at the moment.
We can fix this by not adding this delay for quick start snackbars with action buttons, where we immediately want to show the next task snackbar. I'll shortly submit a PR.
this makes sense if its ok with us. I'd also be ok if we could delay the blue dot to only appear when the Snackbar does, if we need an alternative. But what you described sounds good to me.
with action button (shown for custom duration = 7secs). In the previous implementation, it was 10 secs.
Why did we decrease from 10 to 7?
without action button (shown for default duration ~3secs). This is the same duration that was set for snack bars on quick start dynamic cards (or the newly styled snackbars used all across the app). We might want to increase the duration for these quick start snackbars.
Yeah I can imagine the only issue if we increase this, is it will make the first problem at the top of this comment worse. If we have a success message, and have to wait for 10 seconds before we show the next thing. Let's keep this default for now, and evaluate again when we have the 7->10 change on those other ones. WDYT?
The textColor is not white but colorOnSurface (to account for light/dark), the icon itself is a white drawable.
I would take white to = colorOnSurface in what I was saying.
The other text areas that look white are jsut wrapped in a block.
Is it bad practice to do this? If I wanted the bold text at the start of those longer pieces of text to be bold and colorOnSurface?
I could easily see this as a congratulations bottom sheet (like the post reminders) popup. Setting a local reminder to remove the card would be more involved, but IMO that particular card takes up a fair amount of ATF space and should be removed when complete.
I'll take note to revisit this in future phase. Agree a success message may be a more appropriate solution than retaining the card. Although I just imagine a person who might want to revisit a step. But, might be more related to turning on / off cards...a more general issue to look at in this page if a user can remove cards using the elipsis menu. Another I'll note for myself.
After some more research, this is also a pre-existing condition. :sigh . So not going to address those at the moment.
Quick Start never plays nice!
@osullivanchris
Why did we decrease from 10 to 7?
The old snackbar had a "Not Now" dismiss button and it looked more like a dialog (without auto-dismiss) so 10 secs duration seemed fine earlier. The new ones do not have the dismiss button (can be dismissed using swipe-to-dismiss gesture), 10 secs duration seems too much for a snackbar, however I didn't want to reduce it too much so that users get a chance to hit the "Go" button and not deviate too much from the old duration. We can switch from 7->10, let me know if I should go ahead.
Yeah I can imagine the only issue if we increase this, is it will make the first problem at the top of this comment worse. If we have a success message, and have to wait for 10 seconds before we show the next thing. Let's keep this default for now, and evaluate again when we have the 7->10 change on those other ones. WDYT?
Sounds good!
@ashiagr trying the new build in #15265 the timings feel totally fine! I think we can keep it the way it is for now. Probably due to some of the other weird issues we were having I just felt timings were wrong but couldn't put my finger on why. Now, it feels like I would expect.
We have decided to park
Colors:
I wonder if we should capture those somewhere. I have definitely raised it before in Calls for Testing but maybe it never got addressed.
@osullivanchris, captured above in #15296.
Great, thanks @ashiagr !
Context
Issues Found
Text Styes:
Dead pixels above Snackbar
Area hidden after Snackbar dismisses
Delayed Snackbars
Snackbars disappear quite quickly
End of tour