Page MenuHomePhabricator
Differential D11873

[native] introduce getFarcasterAccountAlreadyLinkedAlertDetails
ClosedPublic
Actions

Authored by ginsu on May 3 2024, 11:38 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 29, 4:42 PM
Unknown Object (File)
Fri, Nov 29, 3:31 PM
Unknown Object (File)
Thu, Nov 21, 9:59 PM
Unknown Object (File)
Tue, Nov 12, 11:25 AM
Unknown Object (File)
Tue, Nov 12, 8:07 AM
Unknown Object (File)
Tue, Nov 12, 8:05 AM
Unknown Object (File)
Sun, Nov 10, 11:42 AM
Unknown Object (File)
Fri, Nov 8, 7:16 PM
View All 58 Files
Subscribers
ashoat
tomek

Details

Reviewers
ashoat
Commits
rCOMM3199cb8164c4: [native] introduce getFarcasterAccountAlreadyLinkedAlertDetails
Summary

Based on feedback in D11863, I want to factor out the logic for this error message since it will be needed in several places.

Test Plan

flow + confirmed the error message was the same

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.May 3 2024, 11:38 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptMay 3 2024, 11:38 PM
ginsu added a child revision: D11863: [lib/native] introduce useLinkFID hook.May 3 2024, 11:38 PM
ginsu added inline comments.May 3 2024, 11:47 PM
native/utils/alert-messages.js
34 ↗(On Diff #39779)

Took a second to have all of these be exported in the bottom of the file, which is a more common pattern I see in our codebase

ginsu added a reviewer: ashoat.May 3 2024, 11:47 PM
Harbormaster completed remote builds in B28653: Diff 39779.May 3 2024, 11:54 PM
ginsu requested review of this revision.May 3 2024, 11:54 PM
ashoat accepted this revision.May 4 2024, 11:19 AM
ashoat added inline comments.
native/account/registration/connect-farcaster.react.js
114 ↗(On Diff #39779)

Just wondering, why rename body with message?

native/utils/alert-messages.js
55 ↗(On Diff #39779)

Should all of these start with a lowercase letter? I thought we only use uppercase for React components (and sometimes all caps for constants)

This revision is now accepted and ready to land.May 4 2024, 11:19 AM
ginsu mentioned this in D12058: [native] fix casing of error alert details.May 15 2024, 11:13 AM
ginsu added inline comments.May 15 2024, 11:15 AM
native/account/registration/connect-farcaster.react.js
114 ↗(On Diff #39779)

I wanted this to match the AlertDetails type. However, I should have also updated the queuedAlert state to use this type too. Will make this update

native/utils/alert-messages.js
55 ↗(On Diff #39779)

D12058

ginsu updated this revision to Diff 40240.May 15 2024, 11:15 AM

address comments + rebase before landing

Harbormaster completed remote builds in B28956: Diff 40240.May 15 2024, 11:35 AM
Closed by commit rCOMM3199cb8164c4: [native] introduce getFarcasterAccountAlreadyLinkedAlertDetails (authored by ginsu). · Explain WhyMay 15 2024, 11:51 AM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMM3199cb8164c4: [native] introduce getFarcasterAccountAlreadyLinkedAlertDetails.
ginsu mentioned this in rCOMM5752a75f79be: [native] fix casing of error alert details.May 15 2024, 12:15 PM

Revision Contents
Changeset List

PathSize
native/
account/
registration/
22 lines
utils/
30 lines

Diff 40240

View Options

native/account/registration/connect-farcaster.react.js

Loading...
View Options

native/utils/alert-messages.js

Loading...