Skip to content

Commit

Permalink
fix: add missing isPortfolioUrl import and update portfolio url name (#…
Browse files Browse the repository at this point in the history
…9243)

## **Description**

Fixes #9242 and
#9244

More specifically, fixes these 2 issues:

- URL not called properly for Stake button since constant name changes
(broken Portfolio Stake link)
- `isPortfolioUrl` is not imported in app/components/UI/Tokens/index.tsx
(broken Portfolio home link)

**NOTE**: These [breaking
issues](#8674) should
have been caught by the typechecker which is [currently disabled for the
components
folder](https://github.com/MetaMask/metamask-mobile/blob/ef475b788f9be7f0d2b66ea749045772ed899ba0/tsconfig.lint.json#L11).
I'd strongly advise that this is re-implemented as these issues could
have been easily been caught at the commit or CI level.

## **Related issues**

Fixes: #9242 and
#9244

## **Manual testing steps**

- Go to Token Listing
- Verify that the new staking icon shows up on ETH on mainnet
- Verify that onPress, the MM browser opens up the Portfolio stake page
with the correct query params
(https://portfolio.metamask.io/stake?metamaskEntry=mobile)
- Verify that if a browser tab is already open on the stake page, the MM
browser picks this tab rather than opening a new one
- Verify that the Token Listing Portfolio button opens up the portfolio
page (https://portfolio.metamask.io?metamaskEntry=mobile) and does not
crash the app

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**


https://github.com/MetaMask/metamask-mobile/assets/22345430/05d258d8-e962-4297-9e2d-b6f7a47f688f

### **After**


https://github.com/MetaMask/metamask-mobile/assets/22345430/8b7cdca4-1028-45e3-8da8-074d80e70755

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.
  • Loading branch information
ameliejyc authored Apr 16, 2024
1 parent 0e1d3d1 commit a8dd6e3
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
14 changes: 13 additions & 1 deletion app/components/UI/Tokens/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,18 @@ describe('Tokens', () => {

expect(getByTestId(PORTFOLIO_BUTTON)).toBeDefined();
});
it('navigates to Portfolio url when portfolio button is pressed', () => {
const { getByTestId } = renderComponent(initialState);

fireEvent.press(getByTestId(PORTFOLIO_BUTTON));
expect(mockNavigate).toHaveBeenCalledWith(Routes.BROWSER.HOME, {
params: {
newTabUrl: `${AppConstants.PORTFOLIO.URL}/?metamaskEntry=mobile`,
timestamp: 123,
},
screen: Routes.BROWSER.VIEW,
});
});
it('should display unable to find conversion rate', async () => {
const state = {
engine: {
Expand Down Expand Up @@ -240,7 +252,7 @@ describe('Tokens', () => {
fireEvent.press(getByTestId(STAKE_BUTTON));
expect(mockNavigate).toHaveBeenCalledWith(Routes.BROWSER.HOME, {
params: {
newTabUrl: `${AppConstants.PORTFOLIO_URL}/stake?metamaskEntry=mobile`,
newTabUrl: `${AppConstants.STAKE.URL}?metamaskEntry=mobile`,
timestamp: 123,
},
screen: Routes.BROWSER.VIEW,
Expand Down
8 changes: 4 additions & 4 deletions app/components/UI/Tokens/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ import ButtonIcon, {
} from '../../../../app/component-library/components/Buttons/ButtonIcon';
import Box from '../../UI/Ramp/components/Box';
import SheetHeader from '../../../../app/component-library/components/Sheet/SheetHeader';
import { isPortfolioUrl } from '../../../../app/util/url';

const Tokens: React.FC<TokensI> = ({ tokens }) => {
const { colors } = useTheme();
Expand Down Expand Up @@ -220,16 +221,15 @@ const Tokens: React.FC<TokensI> = ({ tokens }) => {

const renderStakeButton = (asset: TokenI) => {
const onStakeButtonPress = () => {
const STAKE_URL = `${AppConstants.PORTFOLIO_URL}/stake`;
const existingStakeTab = browserTabs.find((tab: BrowserTab) =>
tab.url.includes(STAKE_URL),
tab.url.includes(AppConstants.STAKE.URL),
);
let existingTabId;
let newTabUrl;
if (existingStakeTab) {
existingTabId = existingStakeTab.id;
} else {
newTabUrl = `${STAKE_URL}?metamaskEntry=mobile`;
newTabUrl = `${AppConstants.STAKE.URL}?metamaskEntry=mobile`;
}
const params = {
...(newTabUrl && { newTabUrl }),
Expand All @@ -245,7 +245,7 @@ const Tokens: React.FC<TokensI> = ({ tokens }) => {
location: 'Home Screen',
text: 'Stake',
token_symbol: asset.symbol,
url: STAKE_URL,
url: AppConstants.STAKE.URL,
});
};

Expand Down
3 changes: 3 additions & 0 deletions app/core/AppConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export default {
ACTIVE: true,
URL: `${PORTFOLIO_URL}/bridge`,
},
STAKE: {
URL: `${PORTFOLIO_URL}/stake`,
},
CONNEXT: {
HUB_EXCHANGE_CEILING_TOKEN: 69,
MIN_DEPOSIT_ETH: 0.03,
Expand Down

0 comments on commit a8dd6e3

Please sign in to comment.