chore: disable protect modal in development

[dbrans]

Description

This PR disables the "Protect your wallet" blocking modal when NODE_ENV is 'development'

Related issues

Fixes:

Manual testing steps

1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

• I’ve followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
• I've completed the PR template to the best of my ability
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

• I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
• I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[matthewwalsh0]

Can all this be encapsulated in a util such as isDevBuild assuming one doesn't already exist?

[dbrans]

Great idea. Done.

[tommasini]

I don't know if it's a good idea to hide this always when it's development, we identified recently bug on that modal, if we apply this we will always miss if any bug is present during development
I think we could create a variable for this but one specific
Open to more thoughts

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
66.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[dbrans]

I don't know if it's a good idea to hide this always when it's development, we identified recently bug on that modal, if we apply this we will always miss if any bug is present during development I think we could create a variable for this but one specific Open to more thoughts

@tommasini I think this could be a good improvement: default the modal to off during development. Developers who need to debug it can just comment out a line as they do now. What do you think?

[tommasini]

I think this could be a good improvement: default the modal to off during development. Developers who need to debug it can just comment out a line as they do now. What do you think?

If I understood correctly you are suggesting that the default behaviour is to hide the protect modal to funded not backed up accounts.
I am not sure just because we don't have anything educating the engineers that this was a feature, some of the new engineers probably don't know it, and if we turn this on by default for not showing the protect modal, I'm afraid they can only discover when they create a bug on it! Sorry, I am just trying to think how this could go wrong, If more team members (QA team included) are okay with this, I'm OK with moving this further!

[NicolasMassart]

S: we may not realy need a const on this as it's only used once and in s very specific scope (envs).
I suggest the following:

Suggested change

	const NODE_ENV = 'NODE_ENV';
	// eslint-disable-next-line import/prefer-default-export
	export const isDevelopment = () => process.env[NODE_ENV] === 'development';
	export const isDevelopment = () => process.env['NODE_ENV'] === 'development';

Also prefer default export is not a rule anymore.

[legobeat]

If it's too onerous for devs to force this behavior, likely that's an indication it is also too arduous for many users as well?

If we solve a UX regression for internal users, shouldn't we solve it for other users at the same time?

Could it perhaps be toggled via "Settings" as a runtime setting, rather than as build-time configuration?

[NicolasMassart]

Just going back on this, I have an idea: the issue is currently that there's no way to escape this modal an once you have it you don't have other choice than saving SRP.
What about a button to ignore for some time, like os upgrade, but have this forced after say 10 ignores. That would allow devs enough time to test things.
The modal would show each time we open the foreground the app on the wallet view.
What do you think?