Update snackbars

[micahmo]

Pull Request Description

• We currently have lots of issues showing snackbars on various pages. Sometimes things break and we lose them (#1064). Sometimes we have to create and pass around lots of scaffolds in order for them to appear on the right page when triggered from another page (#1044 and others). Sometimes they appear underneath what you're viewing. (We introduced a whole customState argument for the snackbar method to get around some of this stuff, and it still doesn't always work.) Sometimes the snackbar jumps position depending on whether the bottom nav bar is present. The summary is that snackbars are hard to get exactly right. I'm also not a huge fan of the appearance (where it covers the whole bottom section), which I believe was only changed to fix an issue, not because it looked better!
• This PR switches our snackbar usage to GetX. Their tagline is "Open screens/snackbars/dialogs/bottomSheets without context", which immediately grabbed my attention!
• We might even be able to use GetX for other things in the app, like navigation or simple state management, although its future is a bit unknown. I think it's worth using just for snackbars though!

One downside of this approach is that the snackbar now covers things like the FABs, instead of them intelligently jumping up in response. However, I think this is a reasonable tradeoff for more reliable snackbars, and I think sometimes the jumping was confusing anyway (when I was about to press a button, it would move).

Issue Being Fixed

Issue Number: N/A

Screenshots / Recordings

Here are a couple specific before and after videos demonstrating the improved usability of snackbars. But there should be many other scenarios that this change improves.

Before: Drafts

snackbar_before_1.mp4

After: Drafts

snackbar_after_1.mp4

Before: Error

sbackbar_before_2.mp4

After: Error

snackbar_after_2.mp4

Before: Navigate to Post

In this before example, the snackbar only works when the new post is initiated from the home feed.

snackbar_before_3.mp4

After: Navigate to Post

snackbar_after_3.mp4

Before: Block

In this before example, the snackbar only appears once we've navigated back to the home feed.

snackbar_before_4.mp4

After: Block

snackbar_after_4.mp4

Dark Mode

snackbar_dark.mp4

These are all of the examples I can come up with right now, but I know I've seen the snackbar do other funky stuff!

Checklist

• Did you update CHANGELOG.md?
• Did you use localized strings where applicable?
• Did you add semanticLabels where applicable for accessibility?

[ggichure]

GetX is generally frowned upon.

[micahmo]

Please see the PR description for my rationale behind this change! Especially watch the before/after videos, and see how the code is simplified without having to worry about context and scaffolds! 😊

[ggichure]

For the background color M3 guidelines recommends onInverseSurface

[micahmo]

I think you mean inverseSurface for the background and onInverseSurface for the foreground. That looks fine to me, so I updated.

Light	Dark

[hjiangsu]

Hmm, I'm a bit divided about making this change.

The main reason why I'm divided about this change is due to a couple of reasons:

• We might be adding this package to solve an issue that could be solved by some simple architectural changes (one example I can think of is placing the SnackBars in more appropriate locations based on where they should be shown, and derive the proper context to use)
• It feels to me like GetX is quite controversial amongst the community (some like it, some hate it, etc). Because of this, it can lead to situations where developers with strong opinions of the package might be reluctant from contributing.
• Additionally, GetX seems to be a bit overkill for the issue we're attempting to solve as it attempts to solve a lot of things (state management, dependency injection, routing, localization, theming, etc.). While this can be good in certain situations, it also has drawbacks because it could cause a situation where we're mixing GetX solutions with other solutions.

Instead of making this change, I'd like to give it a shot at re-architecturing some parts of the app to solve the issues with SnackBars, and make this change as a last resort if all else fails! Thoughts?

[micahmo]

Hmm, I'm a bit divided about making this change.

That's perfectly reasonable! I think I will respectfully disagree with your points (explained below), but of course the final call is yours and I'm happy with whatever you decide!

We might be adding this package to solve an issue that could be solved by some simple architectural changes (one example I can think of is placing the SnackBars in more appropriate locations based on where they should be shown, and derive the proper context to use)

I'm not convinced that we can ever have as elegant of a solution, even with a rearchitecture, as long as we're relying on scaffolds. I think there are too many different scenarios to account for and some we might not even think of. For example, I just hit a situation where I was viewing an image, then I got a notification which opened a page, then I navigated to a post, then a community, then I made a new post. The "navigate to post" snackbar appeared all the way back on the main feed under the image, which I just happened to catch. It was already hard enough to make the create post screen show a snackbar on the main feed, and that's really the only scenario where it works. Unless we can replicate their technique, I don't think anything will be as reliable or convenient as GetX.

It feels to me like GetX is quite controversial amongst the community (some like it, some hate it, etc). Because of this, it can lead to situations where developers with strong opinions of the package might be reluctant from contributing.

If you're right, this would be very unfortunate! From what I've seen, most contributors are not Flutter experts and are happy to learn how we've done things and follow our patterns. Also if we isolate our usage to snackbars, then most of their objections shouldn't apply.

Additionally, GetX seems to be a bit overkill for the issue we're attempting to solve as it attempts to solve a lot of things (state management, dependency injection, routing, localization, theming, etc.).

I disagree that it's overkill, as long as we're not using the additional functionality. If it required a huge refactor just to use their snackbars, maybe. But the only other thing I really had to touch was adding a key to our router. Just because the package can do more, doesn't mean it's overkill to use the part we want (unless you mean that by adding such a big package to our project, we'll increase the release size or something).

While this can be good in certain situations, it also has drawbacks because it could cause a situation where we're mixing GetX solutions with other solutions.

I think if we decide not to use GetX for any of the other things that it can do, then we won't run the risk of mixing solutions. It doesn't seem like the kind of thing that can get in accidentally, so we would purposefully not add it ourselves or allow it in PRs. Contributors wouldn't even necessarily know it's there.

And with all that said, I'll leave it at that and let you make the final call. 😊 I'm happy to work on a different solution, especially if we can find something that is equally reliable.

[ggichure]

Instead of making this change, I'd like to give it a shot at re-architecturing some parts of the app to solve the issues with SnackBars, and make this change as a last resort if all else fails! Thoughts?

This will do.

Also toasts will always display on top and don't depend on scaffolds.

[micahmo]

Also toasts will always display on top and don't depend on scaffolds.

We could use toasts for some things! Unfortunately I don't believe they're interactable or super customizable.

[hjiangsu]

I understand where you're coming from! I still do think that with a bit of re-architecturing (and rethinking where our snackbars should reside in), we can fix the issues we've been facing with them. If it does end up that theres a situation where it's very difficult or highly improbable that we can fix a particular issue, then I don't mind using GetX as a final resort.

I think if we decide not to use GetX for any of the other things that it can do, then we won't run the risk of mixing solutions.

I think this is a bit tricky in practice, because some of the functionality the GetX provides isn't necessarily that clear. For example, in their advanced API (https://pub.dev/packages/get#other-advanced-apis), GetX extends on the context with convenience methods.

For example, when going through PRs, it would be difficult to know that context.orientation(), or context.isTablet() is actually a GetX method unless you're really familiar with the methods that context defines.

Just because the package can do more, doesn't mean it's overkill to use the part we want.

I agree to some extent! We don't necessarily have to use other parts of the package, but there are times where it's tempting to use other parts because they provide convenience (and because the function is already available in the package). This, along with what I mentioned above, makes it difficult to distinguish if we are using native methods, or methods that GetX provides (which further locks us into the package if that makes sense).

For example, I just hit a situation where I was viewing an image, then I got a notification which opened a page, then I navigated to a post, then a community, then I made a new post. The "navigate to post" snackbar appeared all the way back on the main feed under the image, which I just happened to catch.

I tried some things to see if we can solve this situation without requiring a separate package, and here are my results (not sure if I matched the exact scenario you were describing, but I tried to be close). This is the commit of the changes: https://github.com/thunder-app/thunder/compare/develop...refactor/snackbar-create-post?w=1

Note: I mocked the CreatePost method so that it doesn't actually create a post, but just fetches an existing post!

snackbar-test.webm

[micahmo]

That mockup looks great, thanks for trying that scenario! Does the snackbar stay on top if you navigate back through the stack?

Also do you think we can make it floating again? I think that looks much better and matches MD3 guidelines. I can't remember why we changed, but I feel like it was related to this issue.

Again, we'll have to be ok with making fairly extensive changes like that for every one of these weird scenarios that we come across.