Page MenuHomePhabricator
Differential D11484

[native] update FarcasterPrompt based on if the fid is set or not
ClosedPublic
Actions

Authored by ginsu on Mar 29 2024, 2:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 21, 2:51 PM
Unknown Object (File)
Wed, Apr 17, 2:03 AM
Unknown Object (File)
Mon, Apr 15, 4:57 PM
Unknown Object (File)
Mon, Apr 15, 2:46 PM
Unknown Object (File)
Thu, Apr 11, 10:35 AM
Unknown Object (File)
Sun, Apr 7, 9:26 PM
Unknown Object (File)
Sun, Apr 7, 1:00 PM
Unknown Object (File)
Sun, Apr 7, 12:01 PM
View All 20 Files
Subscribers
ashoat
tomek

Details

Reviewers
atul
ashoat
Commits
rCOMMb5530714c2a9: [native] update FarcasterPrompt based on if the fid is set or not
Summary

This diff updates the text content of FarcasterPrompt based on wheter or not the user is logged into farcaster or not (we determine this by whether or not the comm user has an fid set)

Depends on D11483

Test Plan

Please see the screenshots below

When fid is set:

IMG_6793.PNG (2×1 px, 112 KB)

When fid is not set:

IMG_6792.PNG (2×1 px, 137 KB)

Confirmed that these changes to FarcasterPrompt do NOT affect the registration flow

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu created this revision.Mar 29 2024, 2:37 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptMar 29 2024, 2:37 PM
Harbormaster completed remote builds in B27832: Diff 38565.Mar 29 2024, 2:52 PM
ginsu requested review of this revision.Mar 29 2024, 2:52 PM
ginsu planned changes to this revision.Mar 29 2024, 3:09 PM
ashoat added a comment.Mar 30 2024, 1:00 PM

Will this affect the experience when FarcasterPrompt is used from the registration flow?

ginsu added a comment.Apr 1 2024, 12:01 PM

Will this affect the experience when FarcasterPrompt is used from the registration flow?

No, we set the fid only at the very end of the registration flow so the text content will only change in the context of the Farcaster account settings screen. I will make sure that my test plan shows this

ginsu requested review of this revision.Apr 1 2024, 12:09 PM
ginsu edited the summary of this revision. (Show Details)
ginsu edited the test plan for this revision. (Show Details)
ginsu added a reviewer: atul.
ginsu added a parent revision: D11483: [native] introduce farcaster account settings profile row.
ginsu attached a referenced file: F1452477: RPReplay_Final1711998516.MP4. (Show Details)
ginsu attached a referenced file: F1452475: IMG_6792.PNG. (Show Details)
ginsu attached a referenced file: F1452474: IMG_6793.PNG. (Show Details)
ginsu added a reviewer: ashoat.

Making @ashoat blocking reviewer since this diff does have copy changes

ginsu added a child revision: D11512: [native] build out UI for farcaster account settings screen.Apr 1 2024, 12:21 PM
atul accepted this revision.Apr 1 2024, 12:51 PM

Looks fine outside of copy being signed off on

native/components/farcaster-prompt.react.js
14 ↗(On Diff #38565)

I'd match capitalization and do fidContext is missing but up to you

ashoat accepted this revision.Tue, Apr 2, 12:26 PM
This revision is now accepted and ready to land.Tue, Apr 2, 12:26 PM
ginsu updated this revision to Diff 38685.Tue, Apr 2, 10:25 PM

address comments

Harbormaster failed remote builds in B27923: Diff 38685!Tue, Apr 2, 10:28 PM
ginsu updated this revision to Diff 38739.Wed, Apr 3, 9:24 AM

rebase before landing + make sure keyserver build ci works

Harbormaster completed remote builds in B27959: Diff 38739.Wed, Apr 3, 9:41 AM
Closed by commit rCOMMb5530714c2a9: [native] update FarcasterPrompt based on if the fid is set or not (authored by ginsu). · Explain WhyWed, Apr 3, 9:49 AM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMMb5530714c2a9: [native] update FarcasterPrompt based on if the fid is set or not.
ashoat added a comment.Wed, Apr 10, 8:31 AM

This diff actually did end up affecting the registration flow. It introduced a very slight regression due to @ginsu's removal of the question mark. @ginsu, can you reply here either with a diff that fixes it, or a task?

ginsu mentioned this in D11621: [native] fix copy in FarcasterPrompt.Wed, Apr 10, 9:44 AM
ginsu added a comment.Wed, Apr 10, 9:46 AM
In D11484#333855, @ashoat wrote:

This diff actually did end up affecting the registration flow. It introduced a very slight regression due to @ginsu's removal of the question mark. @ginsu, can you reply here either with a diff that fixes it, or a task?

My bad! thanks for catching this! D11621 should fix this issue

ginsu mentioned this in rCOMMb6331ad353dd: [native] fix copy in FarcasterPrompt.Mon, Apr 15, 7:32 PM

Revision Contents
Changeset List

PathSize
native/
components/
36 lines

Diff 38743

View Options

native/components/farcaster-prompt.react.js

Loading...