[$500] Profile - "Go to Expensify Classic" and "Help" row is missing external link icon

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4-37.0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Click Profile
3. Scroll down to "Go to Expensify Classic" and "Help" row

Expected Result:

There will be an external link icon for "Go to Expensify Classic" and "Help" row

Actual Result:

"Go to Expensify Classic" and "Help" row is missing external link icon

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f595c73472283630
• Upwork Job ID: 1754875710547341312
• Last Price Increase: 2024-02-06
• Automatic offers:
• s77rt | Reviewer | 28146434

[melvin-bot]

Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01f595c73472283630

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @s77rt (External)

[lanitochka17]

We think that this bug might be related to #wave8-collect-admins
CC @zanyrenney

[MahmudjonToraqulov]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Profile - "Go to Expensify Classic" and "Help" row is missing external link

What is the root cause of that problem?

It's left here. and here.
If we take a look at about page in here:

App/src/pages/settings/AboutPage/AboutPage.tsx

Line 69 in 0eb6d02

translationKey: 'initialSettingsPage.aboutPage.viewTheCode',

App/src/pages/settings/AboutPage/AboutPage.tsx

Line 79 in 0eb6d02

translationKey: 'initialSettingsPage.aboutPage.viewOpenJobs',

These 2 menus "View the Code" and "View open jobs" has iconRight: Expensicons.NewWindow.

What changes do you think we should make in order to solve the problem?

We should add it to this one. and this one as well.

And just pass it 'Menuitem' component:

App/src/pages/settings/InitialSettingsPage.js

Line 245 in 0eb6d02

<MenuItem

As -> " iconRight={item.iconRight}
shouldShowRightIcon={!!item.iconRight}
"

What alternative solutions did you explore? (Optional)

N/A

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The go to expensify classic and help menu doesn't have the external link icon

What is the root cause of that problem?

The menu doesn't have the right icon like in other parts of the app.

App/src/pages/settings/InitialSettingsPage.js

Lines 169 to 175 in 0eb6d02

	translationKey: 'initialSettingsPage.goToExpensifyClassic',
	icon: Expensicons.NewExpensify,
	action: () => {
	Link.openOldDotLink(CONST.OLDDOT_URLS.INBOX);
	},
	link: () => Link.buildOldDotURL(CONST.OLDDOT_URLS.INBOX),
	},

What changes do you think we should make in order to solve the problem?

Add iconRight: Expensicons.NewWindow, to both menus

and then pass the icon here

App/src/pages/settings/InitialSettingsPage.js

Lines 245 to 246 in 0eb6d02

	<MenuItem
	key={`${keyTitle}_${index}`}

iconRight={item.iconRight}
shouldShowRightIcon={!!item.iconRight} <- to show the external icon only

[FitseTLT]

Proposal

Please re-state the problem that we are trying to solve in this issue.

"Go to Expensify Classic" and "Help" row is missing external link icon

What is the root cause of that problem?

We are missing iconRight here

App/src/pages/settings/InitialSettingsPage.js

Lines 168 to 175 in 0eb6d02

	{
	translationKey: 'initialSettingsPage.goToExpensifyClassic',
	icon: Expensicons.NewExpensify,
	action: () => {
	Link.openOldDotLink(CONST.OLDDOT_URLS.INBOX);
	},
	link: () => Link.buildOldDotURL(CONST.OLDDOT_URLS.INBOX),
	},

and here

App/src/pages/settings/InitialSettingsPage.js

Lines 198 to 205 in 0eb6d02

	{
	translationKey: 'initialSettingsPage.help',
	icon: Expensicons.QuestionMark,
	action: () => {
	Link.openExternalLink(CONST.NEWHELP_URL);
	},
	link: CONST.NEWHELP_URL,
	},

What changes do you think we should make in order to solve the problem?

We need to pass in both places

                iconRight: Expensicons.NewWindow,
                shouldShowRightIcon:true

And here we pass

App/src/pages/settings/InitialSettingsPage.js

Lines 263 to 264 in 0eb6d02

	fallbackIcon={item.fallbackIcon}
	brickRoadIndicator={item.brickRoadIndicator}

iconRight={item.iconRight}
shouldShowRightIcon={!!item.iconRight}

What alternative solutions did you explore? (Optional)

[s77rt]

@Expensify/design This was brought up here #33280 (comment) and #33280 (comment). We didn't get a confirmation on whether this is by design or something that should be fixed.

[shawnborton]

Rather than making this any kind of paid bug to fix, can we just add this to the account settings project? cc @kosmydel @mountiny @trjExpensify

[kosmydel]

Rather than making this any kind of paid bug to fix, can we just add this to the account settings project? cc @kosmydel @mountiny @trjExpensify

Yeah, I think we can handle it. We removed it intentionally as designs didn't include those arrows.

So should I include it in one of our PRs?

[dannymcclain]

Yes please do. I think the designs not including it before was an error. The account settings mocks should be correct and show the new-window icon.

[trjExpensify]

Agreed. @kosmydel want to put it in yours? Link it here, and I'll also assign you and I to this issue to track until it's donezo.

[melvin-bot]

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[dannymcclain]

@mountiny This looks like it's correct on staging now, but Melvin is goin' wild 😂

[mountiny]

@dannymcclain its on staging right :D

[trjExpensify]

Hm, so what happened here?

[trjExpensify]

Did Go to Expensify Classic get replaced with "Switch to Expensify Classic" and the external link icon removed?

[kosmydel]

Did Go to Expensify Classic get replaced with "Switch to Expensify Classic" and the external link icon removed?

I think it was changed in this PR.

[trjExpensify]

Gotcha! @shawnborton looks like you were on that PR, not sure if you peaked it though.

1. Are we going to leave it there as the top most row in account settings versus in the general section?
2. Either way on Some initial fixes and code style updates #1, should we add back the external icon?

CC: @roryabraham

[shawnborton]

Oh boy... I think we have some overlap here with the hybrid app project. Maybe @AndrewGable can comment, but I can see where we wanted to keep the Switch to classic towards the top so it's easy to find your way back.

That being said... maybe we don't need the external icon because I don't think we open up a new window when we switch, right? It should just feel like normal app navigation?

[dannymcclain]

That being said... maybe we don't need the external icon because I don't think we open up a new window when we switch, right? It should just feel like normal app navigation?

Yeah if it switches you "in place" then I don't think we need the external icon. Although I guess I could see an argument that you're "leaving" NewDot so... 🤷‍♂️

I don't have super strong feelings.

[trjExpensify]

Ah yeah. but on web/desktop, doesn't it open a new tab versus a redirect to Classic on the same tab. Or is that changing as well?

Maybe @AndrewGable can comment, but I can see where we wanted to keep the Switch to classic towards the top so it's easy to find your way back.

Hm yeah, fair enough. I can see that if you're an existing user being migrated over and don't know what's going on.

[AndrewGable]

I don't have a personal preference on which icon we use, but I would like to keep the "Switch to Expensify Classic" as the first row when we are on HybridApp. I am less passionate about where it is located on non-HybridApp (on web or desktop).

[dannymcclain]

I think I'm voting no "external link" icon.

I don't have a personal preference on which icon we use, but I would like to keep the "Switch to Expensify Classic" as the first row when we are on HybridApp. I am less passionate about where it is located on non-HybridApp (on web or desktop).

Don't we want all the apps to be the same? Or am I misunderstanding something?

[AndrewGable]

We want all apps to be the same, but these buttons will do different things on web vs HybridApp. I'm for keeping it as the first row on both.

[melvin-bot]

This issue has not been updated in over 15 days. @trjExpensify, @s77rt, @mountiny, @kosmydel eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[s77rt]

Let's close this

[mountiny]

From the comments above it seems like we can close. Please reopen if you feel otherwise