Page MenuHomePhabricator
Differential D11662

[native] introduce barebones TagFarcasterChannelNavigator + TagFarcasterChannel screen
ClosedPublic
Actions

Authored by ginsu on Apr 14 2024, 10:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 8, 4:43 PM
Unknown Object (File)
Thu, Nov 7, 9:51 PM
Unknown Object (File)
Sun, Nov 3, 7:29 AM
Unknown Object (File)
Oct 12 2024, 10:04 PM
Unknown Object (File)
Oct 12 2024, 10:04 PM
Unknown Object (File)
Oct 12 2024, 10:04 PM
Unknown Object (File)
Oct 12 2024, 10:04 PM
Unknown Object (File)
Oct 12 2024, 10:04 PM
View All 87 Files
Subscribers
ashoat
tomek
will

Details

Reviewers
atul
inka
Commits
rCOMMfa258611a1ef: [native] introduce barebones TagFarcasterChannelNavigator + TagFarcasterChannel…
Summary

This diff introduces the barebones navigational logic needed for introducing TagFarcasterChannelNavigator and the TagFarcasterChannel screen. Subsquent diffs will handle extending both of these to match the UI/UX with the rest of the app

Linear tasks: https://linear.app/comm/issue/ENG-7538/introduce-tag-farcaster-channel-option-in-the-community-actions-menu + https://linear.app/comm/issue/ENG-7539/introduce-tagfarcasterchannelmodal-screen + https://linear.app/comm/issue/ENG-7540/introduce-navigational-logic-for-tagfarcasterchannelmodal

Test Plan

flow + confirmed that I could navigate to TagFarcasterChannel from community action buttons

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu created this revision.Apr 14 2024, 10:09 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptApr 14 2024, 10:09 PM
ginsu updated this revision to Diff 39102.Apr 14 2024, 10:10 PM

update

ginsu edited the summary of this revision. (Show Details)Apr 14 2024, 10:12 PM
ginsu added reviewers: atul, inka.
ginsu added inline comments.
native/community-settings/tag-farcaster-channel/tag-farcaster-channel.react.js
19 ↗(On Diff #39102)

This will be updated in subsequent diffs

native/components/community-actions-button.react.js
37 ↗(On Diff #39102)

Added theses new lines to space out the code in this file

Harbormaster completed remote builds in B28211: Diff 39101.Apr 14 2024, 10:25 PM
Harbormaster completed remote builds in B28212: Diff 39102.Apr 14 2024, 10:29 PM
ginsu requested review of this revision.Apr 14 2024, 10:29 PM
atul accepted this revision.Apr 15 2024, 4:36 PM

Looks good, let's definitely memoize return value of TagFarcasterChannelNavigator.

native/community-settings/tag-farcaster-channel/tag-farcaster-channel-navigator.react.js
41–50 ↗(On Diff #39102)

Would be good to memoize this since we've got a Navigator and entire screens.

native/components/community-actions-button.react.js
118–120 ↗(On Diff #39102)

Has this copy been signed off on?
Is "Tag" the verb we've decided on to be product facing?

Just want to double check since I don't have context here. (CC @ashoat)

This revision is now accepted and ready to land.Apr 15 2024, 4:36 PM
ashoat added inline comments.Apr 15 2024, 6:37 PM
native/components/community-actions-button.react.js
118–120 ↗(On Diff #39102)

This works, but this is the kind of thing I'd like sign-off for

ginsu added inline comments.Apr 15 2024, 6:59 PM
native/components/community-actions-button.react.js
118–120 ↗(On Diff #39102)

My bad, definitely should have done that. Will be more thoughtful about this

ginsu updated this revision to Diff 39200.Apr 17 2024, 1:00 PM
ginsu edited the summary of this revision. (Show Details)

address comments + update gating logic for showing tag farcaster channel to check fid is present as well

Harbormaster completed remote builds in B28277: Diff 39200.Apr 17 2024, 1:16 PM
ginsu added a subscriber: will.Apr 17 2024, 1:24 PM
ginsu added inline comments.
native/components/community-actions-button.react.js
40–43 ↗(On Diff #39200)

Another usage of fidContext @will

124–125 ↗(On Diff #39200)

Snuck in this change in the latest update of my diff. The change here is that canTagFarcasterChannel also now checks if there is an fid set. Felt like it didn't make sense to be able to access this screen if you aren't connected to farcaster (fid is not set)

ginsu updated this revision to Diff 39205.Apr 17 2024, 10:11 PM

rebase

ginsu added inline comments.Apr 17 2024, 10:14 PM
native/components/community-actions-button.react.js
121–124 ↗(On Diff #39205)

Snuck in this change in the latest update of my diff. The change here is that canTagFarcasterChannel also now checks if there is an fid set and that community is not GENESIS.

Felt like it didn't make sense to be able to access this screen if you aren't connected to farcaster (fid is not set) and tagging GENESIS to a farcaster channel also did not make sense to me

Harbormaster completed remote builds in B28281: Diff 39205.Apr 17 2024, 10:26 PM
ginsu added a child revision: D11682: [native] introduce screen options to tag farcaster channel stack navigator.Apr 17 2024, 11:10 PM
ginsu updated this revision to Diff 39227.Apr 18 2024, 9:23 AM

rebase before landing

This revision was landed with ongoing or failed builds.Apr 18 2024, 9:24 AM
Closed by commit rCOMMfa258611a1ef: [native] introduce barebones TagFarcasterChannelNavigator + TagFarcasterChannel… (authored by ginsu). · Explain Why
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMMfa258611a1ef: [native] introduce barebones TagFarcasterChannelNavigator + TagFarcasterChannel….
Harbormaster completed remote builds in B28297: Diff 39227.Apr 18 2024, 9:40 AM

Revision Contents
Changeset List

PathSize
native/
community-settings/
tag-farcaster-channel/
65 lines
28 lines
components/
35 lines
navigation/
6 lines
9 lines

Diff 39228

View Options

native/community-settings/tag-farcaster-channel/tag-farcaster-channel-navigator.react.js

Loading...
View Options

native/community-settings/tag-farcaster-channel/tag-farcaster-channel.react.js

Loading...
View Options

native/components/community-actions-button.react.js

Loading...
View Options

native/navigation/root-navigator.react.js

Loading...
View Options

native/navigation/route-names.js

Loading...