Disabling Testnet alert banner

[alwx]

temporarily closes the issue of testnet banner (see #19506) by disabling the testnet alert banner and replacing it with a simple toast that is shown when the user is logged in.
also adds a comment why it's done for the future.

Platforms

• Android
• iOS

Areas that maybe impacted

Functional

• 1-1 chats
• public chats
• group chats
• wallet / transactions
• dapps / app browsing
• account recovery
• new account
• user profile updates
• networks
• mailservers
• fleet
• bootnodes

status: ready

[status-im-auto]

Jenkins Builds

Click to see older builds (2)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	e8f98e7	#1	2024-04-23 10:12:33	~4 min	tests	📄log
✔️	e8f98e7	#1	2024-04-23 10:16:12	~8 min	android	🤖apk 📲

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	3ab012f	#3	2024-04-23 10:21:38	~3 min	tests	📄log
✔️	3ab012f	#3	2024-04-23 10:24:45	~7 min	android-e2e	🤖apk 📲
✔️	3ab012f	#3	2024-04-23 10:26:13	~8 min	ios	📱ipa 📲
✔️	3ab012f	#3	2024-04-23 10:26:41	~9 min	android	🤖apk 📲
✔️	fddaf79	#4	2024-04-23 13:10:31	~6 min	tests	📄log
✔️	fddaf79	#4	2024-04-23 13:14:05	~10 min	ios	📱ipa 📲
✔️	fddaf79	#4	2024-04-23 13:15:56	~12 min	android-e2e	🤖apk 📲
✔️	fddaf79	#4	2024-04-23 13:16:18	~12 min	android	🤖apk 📲

[J-Son89]

Did we agree to this? 🤔

Can you include a video/ screenshots of how it looks?

[J-Son89]

At least just do this for the screens where its breaking- wallet looked fine imo

[Parveshdhull]

empty line

[Parveshdhull]

@alwx please remove fixes from PR description as this will close the issue.
Also please remove your assignment so that it can be picked by someone.

[J-Son89]

let's align on this before merging, should we really remove for all screens?
There's some minor issues for our developer experience but does that justify this?

Does this make things better/worse for QA etc?

[alwx]

@J-Son89 we can't easily show it only in Wallet because that banner is a part of a global alert mechanism implemented in status-im.common.alert-banner and shown on top of everything in home-stack which means everything in communities, chats, browser and wallet gets affected.
Showing it only in the Wallet means implementing a new mechanism for that purpose, which we can do if we want, but this PR is meant to simply roll back changes that were made in the Testnet banner introduction PR just because it caused a lot of problems.

[mariia-skrypnyk]

Hi @J-Son89 @Parveshdhull @alwx !
#19773 it's not the only issue that tracks banner issues.
E.g. we have Change Password added and there are also banner issues there. So @clauxx also need to spend time on fixing them in case banner will be present.
You can find examples here: #19474 (comment)

Yes, Testnet banner gives great visibility but it also raise issues and extra time for their fix.

[Parveshdhull]

Thank you @alwx and @mariia-skrypnyk for your feedback and thank you @J-Son89 for raising the point that this needs to be discussed properly before moving forward.

 It caused a lot of problems

I would say this is a little over-exaggeration. If we see issues reported in #19506 they were minor styling issues and needed two lines of fix (- height alert-banners-top-margin) and footer-container-padding.
For debug issues with inspector and pref monitor are nothing new, overlays like the bottom sheet also have the same behavior. Still, I disabled banners for debug builds.

Yes, the Testnet banner gives great visibility but it also raises issues and extra time for this fix.

This is expected. Whenever we build something new, issues are expected and need to be properly tested and fixed. As the alert banner's implementation was sandboxed it missed proper testing phase and now as this is enabled issues are visible. (In idle case these issues should be caught/reported in either of these PRs).

If there are any issues related to alert banner please do report, so we can fix them. As issues mentioned in #19506 are fixed in #19773.

I don't think issues are any problem, they are minor styling issues and can be fixed quickly. The main question should be do we want this feature or not and discuss it with the design team? As far as I remember, we only implemented this functionality because the design team was very keen on this and little persisting.

[Parveshdhull]

So @clauxx also need to spend time on fixing them in case banner will be present.

@clauxx please let me know if you need any help. Also if issue is only after enabling alert banner, please feel free to log it separately, I can checkout later.
From quick look it seems issue might be in line keyboardVerticalOffset (- (safe-area/get-bottom))}

[yevh-berdnyk]

Hi, I just want to add one more detail from the perspective of e2e tests. This banner presence makes some elements on some screens to be hidden so we need to scroll for them and so it makes the tests a little bit longer. Especially having the issue with SauceLabs emulators that the resolution is a way smaller than a usual modern phone, we need to scroll even more often. Of corse it's not a critical issue but I think it can also be pointed here

[Parveshdhull]

Thank you @yevh-berdnyk, I can disable alert banner for e2e builds, will that be ok?

[yevh-berdnyk]

Thank you @yevh-berdnyk, I can disable alert banner for e2e builds, will that be ok?

That will be definitely cool!

[Parveshdhull]

That will be definitely cool!

done

[smohamedjavid]

I agree with @Parveshdhull to hide it only on the debug builds as hiding the banner completely is not a good idea for the end user (There is a feature planned to disable testnet by tapping on the banner). If this PR fixes those issues and we are all in agreement, can we close this PR?

[alwx]

I don't understand why @Parveshdhull decided to work on this despite the discussions we've had. But whatever, I'm closing this then.