This button is shown only when the flag is set. When it is shown, the login buttons are replaced by restore.
https://linear.app/comm/issue/ENG-9678/introduce-a-restore-button
Depends on D13905
Restore enabled
Restore disabled
Details
Details
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
I think it would be best to tuck the restore button behind the login button.
Basically we would show the QR screen, and then there would be a link below it that says something like "No longer have access to your primary device?"
What do you think?
Comment Actions
It sounds like you're proposing a solution where on the initial screen there are two buttons: Sign in and Register. Clicking Sign in opens the QR screen, where there is a No longer have access to your primary device? text. Clicking this link would open the restore screen. Is that correct?
If yes, then it makes a lot of sense to me, because seeing a restore button on the initial screen might be confusing. This solution could be a little more complicated than the current one, because the QR screen is a modal that would modify the state of the logged out modal - but nothing too crazy.
One suggestion I have is to consider whether the whole No longer have access to your primary device? text should be a link. Maybe we can keep it as plain text and introduce a restore button below it? Not sure how this button should look given the design of our QR screen.
Comment Actions
It sounds like you're proposing a solution where on the initial screen there are two buttons: Sign in and Register. Clicking Sign in opens the QR screen, where there is a No longer have access to your primary device? text. Clicking this link would open the restore screen. Is that correct?
Yes, exactly!
One suggestion I have is to consider whether the whole No longer have access to your primary device? text should be a link. Maybe we can keep it as plain text and introduce a restore button below it? Not sure how this button should look given the design of our QR screen.
Hmm... the reason I'm thinking of a link is that our restore flow is comparable to a "Forgot password?" flow, and I usually see those accessed by a link.
I think a button would be too distracting, and might come across as part of the QR code flow when it's really a separate thing.
Would your concerns be addressed if we came up with shorter copy? Maybe "Lost your primary device?"
Comment Actions
Hmm... the reason I'm thinking of a link is that our restore flow is comparable to a "Forgot password?" flow, and I usually see those accessed by a link.
I think a button would be too distracting, and might come across as part of the QR code flow when it's really a separate thing.
Yeah, that makes sense.
Would your concerns be addressed if we came up with shorter copy? Maybe "Lost your primary device?"
I was more concerned about users being able to notice that a link can be clickable. In the mobile app, we usually use buttons to indicate actions that can be performed. There are some usages of LinkButton, but only in the navigation header where users expect some actions to be possible.
Comment Actions
I was more concerned about users being able to notice that a link can be clickable. In the mobile app, we usually use buttons to indicate actions that can be performed. There are some usages of LinkButton, but only in the navigation header where users expect some actions to be possible.
That's a fair concern. It might be possible to improve with color, italics, underline, etc... might be easier to think about if we had a screenshot.
Comment Actions
To be honest, I think it looks okay without the underline. What do you think?
I think it would be good to have the link aligned with the bottom of the "safe area" (just above the home pill).
Separately: realizing from these screenshots that we tell the user to scan the QR code with their phone, not specifically their primary device. I suspect they maybe don't even know what a "primary device" is...
Potentially, we could take the following approach:
- Replace "Lost your primary device?" with something like "Not logged in on your phone?"
- If the QR code screen is opened on mobile, then we should say "logged-in phone" or "primary device" instead of just "phone" in the QR code prompt.
- We should probably have some specific error we're able to show if the user scans a QR code with a secondary device.
Comment Actions
To be honest, I think it looks okay without the underline. What do you think?
Agree, it is clear enough that this is a link
I think it would be good to have the link aligned with the bottom of the "safe area" (just above the home pill).
Separately: realizing from these screenshots that we tell the user to scan the QR code with their phone, not specifically their primary device. I suspect they maybe don't even know what a "primary device" is...
Potentially, we could take the following approach:
- Replace "Lost your primary device?" with something like "Not logged in on your phone?"
Updated on the screenshot
- If the QR code screen is opened on mobile, then we should say "logged-in phone" or "primary device" instead of just "phone" in the QR code prompt.
I'll create a separate diff for it
- We should probably have some specific error we're able to show if the user scans a QR code with a secondary device.
Created https://linear.app/comm/issue/ENG-9936/show-a-specific-error-when-scanning-a-qr-code-on-a-secondary-device to track
Comment Actions
Update the UI. Navigating from the QR screen to the restore flow will be added later in the stack.
Comment Actions
Looks great!
| native/account/logged-out-modal.react.js | ||
|---|---|---|
| 560–572 ↗ | (On Diff #45836) | Something seems wrong here... these lines that are being removed don't seem to appear on master, and don't seem to appear in the parent diff either |
| native/qr-code/qr-code-screen.react.js | ||
| 47 ↗ | (On Diff #45836) | One last comment:
Separately, is the onPress replaced in the next diff? If not we might want to move the no-op function definition to the top-level scope to avoid passing a new one to LinkButton on every render |
Comment Actions
- If the QR code screen is opened on mobile, then we should say "logged-in phone" or "primary device" instead of just "phone" in the QR code prompt.
I'll create a separate diff for it
| native/account/logged-out-modal.react.js | ||
|---|---|---|
| 560–572 ↗ | (On Diff #45836) | Ah, instead of amending I created a new commit - going to fix it. |
| native/qr-code/qr-code-screen.react.js | ||
| 47 ↗ | (On Diff #45836) | This screen is native-specific and there's no web counterpart. On the web, we don't support the restoration flow, so I don't think it makes sense to introduce the link to the restore flow. If we want it, we should make it behave differently, maybe add some explanation that the user can restore their account on a phone... not sure.
I'm going to open a new diff where this is replaced. This is more involved and it makes sense to separate them out. Not sure yet whether this new diff will appear before or after this one, but I'm going to update this diff accordingly. |
Comment Actions
On the web, we don't support the restoration flow, so I don't think it makes sense to introduce the link to the restore flow.
Ahh... yes of course, sorry that should've been obvious 😅