MetaMask / MetaMask/metamask-mobile

Refactor: Consolidate Duplicate Navigation Logic into a Utility Function

Open
#9,113 0 comments 0 reactions 0 assignees View on GitHub
type-refactor
Dominant language
TypeScript
Stars
3k
Forks
1.7k
Avg merge
1d 14h
Merged PRs (30d)
669

Description

### What is this about?

Throughout our codebase, we've identified multiple instances where we're using similar navigation logic to direct users to various webviews. This repetition not only bloats our code but also makes future changes more cumbersome and error-prone. To address this, we propose creating a unified utility function that handles this navigation pattern.

### Scenario

_No response_

### Design

_No response_

### Technical Details

Current Implementation
Currently, the navigation to webviews is handled directly within component methods or lifecycle hooks. An example can be found in [insert specific file path or component name here], where we navigate to the security tips webview:
```
navigation.navigate('Webview', {
screen: 'SimpleWebview',
params: {
url: AppConstants.URLS.SECURITY,
title: strings('add_asset.banners.custom_security_tips'),
},
});
```
This pattern is repeated in several other places within our codebase, leading to duplicated logic.

Proposed Solution
We suggest creating a utility function, potentially within a navigationUtils.js file, that abstracts this common navigation pattern. This function would accept parameters for the URL and title (and any other relevant parameters) and handle the navigation:

```
// In navigationUtils.js
export function navigateToWebview(url, title) {
navigation.navigate('Webview', {
screen: 'SimpleWebview',
params: { url, title },
});
}
```

This approach would allow us to replace all instances of the direct navigation.navigate calls with a single, reusable function call, thereby reducing code duplication and simplifying future updates or changes to our navigation logic.

### Threat Modeling Framework

_No response_

### Acceptance Criteria

_No response_

### Stakeholder review needed before the work gets merged

- [ ] Engineering (needed in most cases)
- [ ] Design
- [ ] Product
- [ ] QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
- [ ] Security
- [ ] Legal
- [ ] Marketing
- [ ] Management (please specify)
- [ ] Other (please specify)

### References

_No response_

Contributor guide

Open the contributing guide

Research direction

The issue mentions repeated navigation.navigate calls and proposes a navigationUtils.js utility, but it does not identify the affected files or tests. Search the codebase for the shown Webview/SimpleWebview navigation pattern and determine the complete set of parameters and call sites. Done means the repeated navigation logic is consolidated without changing the resulting webview behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, react-native
Domain
mobile
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.