Page MenuHomePhabricator
Differential D3825

[web] Turn `ThreadSettingsModal` into functional component
ClosedPublic
Actions

Authored by atul on Apr 24 2022, 12:54 PM.
Tags
None
Referenced Files
F5536549: D3825.id11830.diff
Sat, Apr 12, 3:12 AM
F5536548: D3825.id.diff
Sat, Apr 12, 3:12 AM
Unknown Object (File)
Tue, Apr 8, 10:50 PM
Unknown Object (File)
Tue, Apr 8, 6:09 PM
Unknown Object (File)
Tue, Apr 8, 5:10 PM
Unknown Object (File)
Tue, Apr 8, 4:10 PM
Unknown Object (File)
Tue, Apr 8, 8:21 AM
Unknown Object (File)
Tue, Apr 8, 7:36 AM
View All Files
Subscribers
• abosh
• adrian
ashoat
• karol

Details

Reviewers
• jacek
tomek
Commits
rCOMMc952321494b1: [web] Turn `ThreadSettingsModal` into functional component
Summary

This diff completely removes the remnants of the ThreadSettingsModal class component. Specifically, this diff lifts the render() function of the class component to the return value of the outer functional component.

Now that we've turned ThreadSettingsModal into a functional component, we're going to continue the refactor by pushing "tab-specific" state/functionality to the ThreadSettings[General/Privacy/Delete]Tab child components (which are also now functional components)

Depends on D3822

Test Plan
  1. Open ThreadSettingsModal
  2. Click around, make some changes, hit save
  3. Make sure that everything continues to look/behave as expected

Diff Detail

Repository
rCOMM Comm
Branch
clean (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Apr 24 2022, 12:54 PM
Herald added subscribers: • abosh, • karol, • adrian, ashoat. · View Herald TranscriptApr 24 2022, 12:54 PM
atul added a comment.Apr 24 2022, 12:55 PM

(As of this diff, we've trimmed ThreadSettingsModal from being ~850 lines before refactor => 409 lines... will be even smaller after pushing down state/functionality to the child "tab" components)

atul retitled this revision from [web] Turn `ThreadSettingsModal` into ` functional component to [web] Turn `ThreadSettingsModal` into functional component.Apr 24 2022, 12:57 PM
Harbormaster completed remote builds in B8485: Diff 11830.Apr 24 2022, 12:59 PM
atul requested review of this revision.Apr 24 2022, 12:59 PM
atul added a child revision: D3826: [web] Remove duplicate `thread[Name/Description]Disabled` props in `ThreadSettingsGeneralTab` component.Apr 24 2022, 1:05 PM
tomek accepted this revision.Apr 26 2022, 8:09 AM
In D3825#106086, @atul wrote:

(As of this diff, we've trimmed ThreadSettingsModal from being ~850 lines before refactor => 409 lines... will be even smaller after pushing down state/functionality to the child "tab" components)

Nice!

This revision is now accepted and ready to land.Apr 26 2022, 8:09 AM
atul mentioned this in D3823: [web] Lift `buttons: React.Node` from `ThreadSettingsModal` to `ConnectedThreadSettingsModal`.Apr 26 2022, 10:18 AM
atul updated this revision to Diff 11945.Apr 26 2022, 10:23 AM

rebase

Harbormaster completed remote builds in B8566: Diff 11945.Apr 26 2022, 10:28 AM
atul edited the summary of this revision. (Show Details)Apr 26 2022, 12:25 PM
atul added a parent revision: D3822: [web] Lift `mainContent: React.Node` from `ThreadSettingsModal` to `ConnectedThreadSettingsModal`.
atul updated this revision to Diff 11964.Apr 26 2022, 1:02 PM

rebase

Harbormaster completed remote builds in B8583: Diff 11964.Apr 26 2022, 1:07 PM
atul updated this revision to Diff 12068.Apr 28 2022, 1:11 PM

rebase before landing

Harbormaster completed remote builds in B8668: Diff 12068.Apr 28 2022, 1:16 PM
Closed by commit rCOMMc952321494b1: [web] Turn `ThreadSettingsModal` into functional component (authored by atul). · Explain WhyApr 28 2022, 1:18 PM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMc952321494b1: [web] Turn `ThreadSettingsModal` into functional component.

Revision Contents
Changeset List

PathSize
web/
modals/
threads/
115 lines

Diff 11945

View Options

web/modals/threads/thread-settings-modal.react.js

Show First 20 Lines • Show All 147 Lines • ▼ Show 20 Lines `${permission} can be disabled by a block, but threadHasPermission can't ` +
'check for a block on RawThreadInfo. Please pass in ThreadInfo instead!', 'check for a block on RawThreadInfo. Please pass in ThreadInfo instead!',
); );
if (threadInfo.minimallyEncoded) { if (threadInfo.minimallyEncoded) {
return hasPermission(threadInfo.currentUser.permissions, permission); return hasPermission(threadInfo.currentUser.permissions, permission);
} }
return permissionLookup(threadInfo.currentUser.permissions, permission); return permissionLookup(threadInfo.currentUser.permissions, permission);
} }
function useThreadHasPermission( function useThreadsWithPermission(
threadInfo: ?ThreadInfo, threadInfos: $ReadOnlyArray<ThreadInfo>,
permission: ThreadPermission, permission: ThreadPermission,
): boolean { ): $ReadOnlyArray<ThreadInfo> {
const loggedInUserInfo = useLoggedInUserInfo(); const loggedInUserInfo = useLoggedInUserInfo();
const userInfos = useSelector(state => state.userStore.userInfos); const userInfos = useSelector(state => state.userStore.userInfos);
return React.useMemo(() => { return React.useMemo(() => {
if (!threadInfo) { return threadInfos.filter((threadInfo: ThreadInfo) => {
return false;
}
const permissions = threadFrozenDueToBlock( const permissions = threadFrozenDueToBlock(
threadInfo, threadInfo,
loggedInUserInfo?.id, loggedInUserInfo?.id,
userInfos, userInfos,
) )
? filterOutDisabledPermissions(threadInfo.currentUser.permissions) ? filterOutDisabledPermissions(threadInfo.currentUser.permissions)
: threadInfo.currentUser.permissions; : threadInfo.currentUser.permissions;
return hasPermission(permissions, permission); return hasPermission(permissions, permission);
}, [loggedInUserInfo?.id, permission, threadInfo, userInfos]); });
}, [loggedInUserInfo?.id, permission, threadInfos, userInfos]);
}
function useThreadHasPermission(
threadInfo: ?ThreadInfo,
permission: ThreadPermission,
): boolean {
return (
useThreadsWithPermission(threadInfo ? [threadInfo] : [], permission)
.length === 1
);
ashoatUnsubmitted
Done
Inline Actions
): boolean {
- return (
- useThreadsWithPermission(threadInfo ? [threadInfo] : [], permission)
- .length === 1
- );
+ const threads = useThreadsWithPermission(threadInfo ? [threadInfo] : [], permission);
+ return threads.length === 1;
}
function viewerIsMember(

Can we extract this to its own line? Similar to how we want await to be visible, I think it's good for hooks to be as well

ashoat: Can we extract this to its own line? Similar to how we want `await` to be visible, I think it's…
} }
function viewerIsMember( function viewerIsMember(
threadInfo: ?(ThreadInfo | LegacyRawThreadInfo | RawThreadInfo), threadInfo: ?(ThreadInfo | LegacyRawThreadInfo | RawThreadInfo),
): boolean { ): boolean {
return !!( return !!(
threadInfo && threadInfo &&
threadInfo.currentUser.role !== null && threadInfo.currentUser.role !== null &&
▲ Show 20 Lines • Show All 1,812 Lines • Show Last 20 Lines