Changeset View
Changeset View
Standalone View
Standalone View
web/modals/threads/create/steps/subchannel-members-list.react.js
- This file was added.
// @flow | |||||
import * as React from 'react'; | |||||
import { useSelector } from 'react-redux'; | |||||
import type { ThreadInfo } from 'lib/types/thread-types'; | |||||
import { trimText } from 'lib/utils/text-utils'; | |||||
import AddMembersList from '../../../components/add-members-list.react'; | |||||
type Props = { | |||||
+searchText: string, | |||||
+searchResult: $ReadOnlySet<string>, | |||||
+communityThreadInfo: ThreadInfo, | |||||
+parentThreadInfo: ThreadInfo, | |||||
+selectedUsers: $ReadOnlySet<string>, | |||||
+switchUser: (userID: string) => void, | |||||
}; | |||||
function Memberlist(props: Props): React.Node { | |||||
const { | |||||
searchText, | |||||
searchResult, | |||||
communityThreadInfo, | |||||
parentThreadInfo, | |||||
selectedUsers, | |||||
switchUser, | |||||
} = props; | |||||
const { | |||||
members: communityMembers, | |||||
name: communityName, | |||||
} = communityThreadInfo; | |||||
const currentUserId = useSelector(state => state.currentUserInfo.id); | |||||
const parentMembers = React.useMemo( | |||||
() => new Set(parentThreadInfo.members.map(user => user.id)), | |||||
[parentThreadInfo], | |||||
); | |||||
const parentMemberList = React.useMemo( | |||||
() => | |||||
communityMembers.filter( | |||||
tomek: This might be really confusing. In this component we handle two thread infos: one for parent… | |||||
jakubAuthorUnsubmitted Done Inline ActionsMembers from parentThreadInfo are also community members, but we could also use parenThreadInfo.members inside parentMembersList instead of communityMembers. jakub: Members from `parentThreadInfo` are also community members, but we could also use… | |||||
user => | |||||
parentMembers.has(user.id) && | |||||
user.id !== currentUserId && | |||||
(searchResult.has(user.id) || searchText.length === 0), | |||||
), | |||||
[communityMembers, parentMembers, currentUserId, searchResult, searchText], | |||||
); | |||||
const otherMemberList = React.useMemo( | |||||
() => | |||||
communityMembers.filter( | |||||
user => | |||||
!parentMembers.has(user.id) && | |||||
atulUnsubmitted Done Inline ActionsDo we not want to include the user.id !== currentUserId check from above here as well? atul: Do we not want to include the `user.id !== currentUserId` check from above here as well? | |||||
jakubAuthorUnsubmitted Done Inline ActionscurrentUserId is currently included in parentMembers set. For better code maintenance, I could include this line. jakub: `currentUserId` is currently included in `parentMembers` set. For better code maintenance, I… | |||||
(searchResult.has(user.id) || searchText.length === 0), | |||||
), | |||||
[communityMembers, parentMembers, searchResult, searchText], | |||||
); | |||||
const sortedGroupedUserList = React.useMemo( | |||||
() => | |||||
[ | |||||
{ header: 'Users in parent channel', userInfos: parentMemberList }, | |||||
{ | |||||
header: `All users in ${ | |||||
trimText(communityName || '', 22) ?? '<not specified>' | |||||
tomekUnsubmitted Done Inline ActionsWhy do we use a magic number here? A lot better approach would be to have the whole text here and UI could determine the length using e.g. text-overflow css tomek: Why do we use a magic number here? A lot better approach would be to have the whole text here… | |||||
jakubAuthorUnsubmitted Done Inline ActionsSure, this approach requires modifications in AddMemberGroup css. For now, I could remove it and create separate diff for that. jakub: Sure, this approach requires modifications in `AddMemberGroup` css. For now, I could remove it… | |||||
tomekUnsubmitted Not Done Inline ActionsMakes sense tomek: Makes sense | |||||
}`, | |||||
userInfos: otherMemberList, | |||||
}, | |||||
].filter(item => item.userInfos.length), | |||||
[parentMemberList, otherMemberList, communityName], | |||||
); | |||||
return ( | |||||
<AddMembersList | |||||
switchUser={switchUser} | |||||
pendingUsersToAdd={selectedUsers} | |||||
sortedGroupedUsersList={sortedGroupedUserList} | |||||
/> | |||||
); | |||||
} | |||||
export default Memberlist; |
This might be really confusing. In this component we handle two thread infos: one for parent and one for community. To determine parentMemberList we use communityMembers which is really confusing because a reader would expect that they are taken from parentThreadInfo. Maybe this is an intention and only the naming should be improved, but it might also be the case that the logic is invalid.