Skip to content

fix: Added web link for desktop error page#7364

Merged
NikkiWines merged 3 commits into
Expensify:mainfrom
mananjadhav:fix/desktop-error-page-link
Jan 31, 2022
Merged

fix: Added web link for desktop error page#7364
NikkiWines merged 3 commits into
Expensify:mainfrom
mananjadhav:fix/desktop-error-page-link

Conversation

@mananjadhav

Copy link
Copy Markdown
Collaborator

Details

  • Added web link for desktop error page

Fixed Issues

$ #7316

Tests

  1. Tested link for the Desktop App
  • Verify that no errors appear in the JS console

QA Steps

Need to follow the reproduction steps from #6856 (comment) for Desktop.

  • Verify that no errors appear in the JS console

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web-mobile-web-link

Mobile Web

Desktop

desktop-link

iOS

Android

@mananjadhav mananjadhav requested a review from a team as a code owner January 21, 2022 22:17
@MelvinBot MelvinBot requested review from NikkiWines and parasharrajat and removed request for a team January 21, 2022 22:17
parasharrajat
parasharrajat previously approved these changes Jan 21, 2022

@parasharrajat parasharrajat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

cc: @NikkiWines
🎀 👀 🎀 C+ reviewed

Comment thread src/CONST.js Outdated
@parasharrajat

Copy link
Copy Markdown
Member

Thanks for correcting the link. I missed that.

@NikkiWines

Copy link
Copy Markdown
Contributor

Just a note that the linked QA steps don't work currently on dev - looks like someone fixed the room details issue. I would update the QA steps to direct the tester to go to a malformed URL like https://new.expensify.com/r/75431276____/details

@mananjadhav

Copy link
Copy Markdown
Collaborator Author

Just a note that the linked QA steps don't work currently on dev - looks like someone fixed the room details issue. I would update the QA steps to direct the tester to go to a malformed URL like https://new.expensify.com/r/75431276____/details

@NikkiWines Anyway to get this working for Desktop App? Because we can't enter URL there.

@NikkiWines

Copy link
Copy Markdown
Contributor

@mananjadhav oh, right - in that case if we can use the following steps for desktop (this flow works fine on web, oddly):

  1. Create a new room using the + FAB
  2. Click on the room's icon to open up the room details
  3. Click on Settings
  4. Confirm you see the error page and that it directs you to switch to web
Screen.Recording.2022-01-24.at.10.17.39.mov

@NikkiWines NikkiWines left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@NikkiWines

Copy link
Copy Markdown
Contributor

@mananjadhav since you mentioned tackling the url constant updates in a separate issue/PR, I'm going to go ahead and merge this - sound good?

@mananjadhav

Copy link
Copy Markdown
Collaborator Author

Yeah thanks, will you also help create an issue then? It'll be easier to track?

@NikkiWines

Copy link
Copy Markdown
Contributor

@roryabraham created one already here

@NikkiWines NikkiWines merged commit 7f0e7f8 into Expensify:main Jan 31, 2022
@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify

OSBotify commented Feb 1, 2022

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @NikkiWines in version: 1.1.33-4 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify

OSBotify commented Feb 2, 2022

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by @sketchydroide in version: 1.1.34-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants