Page MenuHomePhabricator

[lib/web] Route to a barebones QR code login page
ClosedPublic

Authored by rohan on Aug 15 2023, 10:11 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, May 5, 1:35 AM
Unknown Object (File)
Wed, Apr 24, 6:43 PM
Unknown Object (File)
Wed, Apr 24, 6:43 PM
Unknown Object (File)
Wed, Apr 24, 6:43 PM
Unknown Object (File)
Wed, Apr 24, 6:39 PM
Unknown Object (File)
Wed, Apr 24, 2:37 PM
Unknown Object (File)
Mon, Apr 22, 3:35 PM
Unknown Object (File)
Sun, Apr 21, 11:40 PM
Subscribers

Details

Summary

While the QR code work is in progress, we don't want it to be the first login page displayed. Instead, we're hiding it behind a 'login via QR code button' as done earlier in the stack.

The context is in ENG-4585, but the basics for this diff are:

  • Update NavInfo to include a loginMethod: form | qr-code
  • Update URLInfo to include a qrCode?: boolean prop
  • Add the RegEx statement alongside the other url regex statements to match for qr-code in the URL
  • In web/url-utils.js, we should do two things:
    • in canonicalURLFromReduxState: append 'qr-code' to the url if the user is not logged in and navInfo.loginMethod === 'qr-code'
    • in navInfoFromURL: if urlInfo.qrCode === true, we want to set newNavInfo.loginMethod === 'qr-code', otherwise we set it to 'form'
  • Lastly, in web/app.react.js, instead of just rendering Splash when the user is not logged in, we look at the navInfo and decide whether to render Splash or the new page that will display the QR code (this logic is pretty similar to what is done for renderMainContent()

Mainly followed by the settings stack (D3342) for inspiration on writing this code

Depends on D8811

Test Plan

Confirmed the following:

  1. The main login page is still the form
  2. Clicking the 'login via qr code button' navigates to the new barebones page and the url updates

(I reload the page first to show that the form is still rendered by default, then click the button to render what will be the QR code page)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan edited the test plan for this revision. (Show Details)
rohan edited the test plan for this revision. (Show Details)
tomek requested changes to this revision.Aug 16 2023, 3:21 AM

It might be a good idea to also check that:

  1. Pasting an URL with a QR code opens the right screen
  2. Going back to a QR code screen opens it
  3. Refreshing a QR code screen opens it
  4. Opening a QR code screen as a logged-in user by pasting a URL doesn't open the screen (do we want that?)
web/app.react.js
215–225 ↗(On Diff #29914)

It doesn't seem to replicate the original behavior. In the previous version, we were always showing Splash. Now it is possible to have undefined.

web/url-utils.js
73–75 ↗(On Diff #29914)

How about using else-if?

76 ↗(On Diff #29914)

Shouldn't we put a slash at the end? Also, we can use straight quotes.

This revision now requires changes to proceed.Aug 16 2023, 3:21 AM
In D8818#260415, @tomek wrote:

It might be a good idea to also check that:

  1. Pasting an URL with a QR code opens the right screen
  2. Going back to a QR code screen opens it
  3. Refreshing a QR code screen opens it
  4. Opening a QR code screen as a logged-in user by pasting a URL doesn't open the screen (do we want that?)

Checked all of these and they all work fine! Regarding #4, when logged in the url redirects to the user's chats and that behavior makes sense to me

This revision is now accepted and ready to land.Aug 17 2023, 4:28 AM
web/types/nav-types.js
41

You failed to update the navInfoValidator, causing ENG-4666