fix: address PR reviews and optimize search results

This commit is contained in:
Audric Ackermann 2023-06-23 14:03:52 +02:00
parent 0e18bf4640
commit fed58161a0
13 changed files with 131 additions and 148 deletions

View file

@ -13,6 +13,7 @@ import { isMessageSelectionMode } from '../../state/selectors/conversations';
import { SessionIcon } from '../icon';
import { AvatarPlaceHolder } from './AvatarPlaceHolder/AvatarPlaceHolder';
import { ClosedGroupAvatar } from './AvatarPlaceHolder/ClosedGroupAvatar';
import { isEqual } from 'lodash';
export enum AvatarSize {
XS = 28,
@ -33,7 +34,7 @@ type Props = {
dataTestId?: string;
};
const Identicon = (props: Props) => {
const Identicon = (props: Pick<Props, 'forcedName' | 'pubkey' | 'size'>) => {
const { size, forcedName, pubkey } = props;
const displayName = useConversationUsername(pubkey);
const userName = forcedName || displayName || '0';
@ -81,15 +82,15 @@ const NoImage = (
return <Identicon size={size} forcedName={forcedName} pubkey={pubkey} />;
};
const AvatarImage = (props: {
avatarPath?: string;
base64Data?: string;
name?: string; // display name, profileName or pubkey, whatever is set first
imageBroken: boolean;
datatestId?: string;
handleImageError: () => any;
}) => {
const { avatarPath, base64Data, imageBroken, datatestId, handleImageError } = props;
const AvatarImage = (
props: Pick<Props, 'base64Data' | 'dataTestId'> & {
avatarPath?: string;
name?: string; // display name, profileName or pubkey, whatever is set first
imageBroken: boolean;
handleImageError: () => any;
}
) => {
const { avatarPath, base64Data, imageBroken, dataTestId, handleImageError } = props;
const disableDrag = useDisableDrag();
@ -104,7 +105,7 @@ const AvatarImage = (props: {
onError={handleImageError}
onDragStart={disableDrag}
src={dataToDisplay}
data-testid={datatestId}
data-testid={dataTestId}
/>
);
};
@ -163,7 +164,7 @@ const AvatarInner = (props: Props) => {
imageBroken={imageBroken}
name={forcedName || name}
handleImageError={handleImageError}
datatestId={dataTestId ? `img-${dataTestId}` : undefined}
dataTestId={dataTestId ? `img-${dataTestId}` : undefined}
/>
) : (
<NoImage {...props} isClosedGroup={isClosedGroupAvatar} />
@ -172,4 +173,4 @@ const AvatarInner = (props: Props) => {
);
};
export const Avatar = AvatarInner;
export const Avatar = React.memo(AvatarInner, isEqual);

View file

@ -336,13 +336,16 @@ export const MessageContextMenu = (props: Props) => {
<Item onClick={onDeleteForEveryone}>{unsendMessageText}</Item>
</>
) : null}
{showAdminActions ? <Item onClick={onBan}>{window.i18n('banUser')}</Item> : null}
{showAdminActions ? <Item onClick={onUnban}>{window.i18n('unbanUser')}</Item> : null}
{showAdminActions && !isSenderAdmin ? (
<Item onClick={addModerator}>{window.i18n('addAsModerator')}</Item>
) : null}
{showAdminActions && isSenderAdmin ? (
<Item onClick={removeModerator}>{window.i18n('removeFromModerators')}</Item>
{showAdminActions ? (
<>
<Item onClick={onBan}>{window.i18n('banUser')}</Item>
<Item onClick={onUnban}>{window.i18n('unbanUser')}</Item>
{isSenderAdmin ? (
<Item onClick={removeModerator}>{window.i18n('removeFromModerators')}</Item>
) : (
<Item onClick={addModerator}>{window.i18n('addAsModerator')}</Item>
)}
</>
) : null}
</Menu>
</SessionContextMenuContainer>

View file

@ -122,7 +122,7 @@ const animation = (props: {
};
//tslint:disable no-unnecessary-callback-wrapper
const Svg = styled.svg<StyledSvgProps>`
const Svg = React.memo(styled.svg<StyledSvgProps>`
width: ${props => props.width};
transform: ${props => `rotate(${props.iconRotation}deg)`};
animation: ${props => animation(props)};
@ -134,7 +134,7 @@ const Svg = styled.svg<StyledSvgProps>`
fill: ${props => (props.iconColor ? props.iconColor : '--button-icon-stroke-color')};
padding: ${props => (props.iconPadding ? props.iconPadding : '')};
transition: inherit;
`;
`);
// tslint:enable no-unnecessary-callback-wrapper
const SessionSvg = (props: {

View file

@ -3,8 +3,8 @@ import React from 'react';
import { useSelector } from 'react-redux';
import styled from 'styled-components';
import { SectionType } from '../../state/ducks/section';
import { getLeftPaneLists } from '../../state/selectors/conversations';
import { getSearchResults, isSearching } from '../../state/selectors/search';
import { getLeftPaneConversationIds } from '../../state/selectors/conversations';
import { getSearchResultsIdsOnly, isSearching } from '../../state/selectors/search';
import { getFocusedSection, getOverlayMode } from '../../state/selectors/section';
import { SessionTheme } from '../../themes/SessionTheme';
import { SessionToastContainer } from '../SessionToastContainer';
@ -24,16 +24,16 @@ const StyledLeftPane = styled.div`
const InnerLeftPaneMessageSection = () => {
const showSearch = useSelector(isSearching);
const searchResults = showSearch ? useSelector(getSearchResults) : undefined;
const searchResults = useSelector(getSearchResultsIdsOnly);
const conversations = showSearch ? undefined : useSelector(getLeftPaneLists);
const conversationIds = useSelector(getLeftPaneConversationIds);
const overlayMode = useSelector(getOverlayMode);
return (
// tslint:disable-next-line: use-simple-attributes
<LeftPaneMessageSection
conversations={conversations || []}
searchResults={searchResults}
conversationIds={showSearch ? undefined : conversationIds || []}
searchResults={showSearch ? searchResults : undefined}
overlayMode={overlayMode}
/>
);

View file

@ -4,10 +4,6 @@ import { AutoSizer, List, ListRowProps } from 'react-virtualized';
import { SearchResults, SearchResultsProps } from '../search/SearchResults';
import { LeftPaneSectionHeader } from './LeftPaneSectionHeader';
import { MessageRequestsBanner } from './MessageRequestsBanner';
import {
ConversationListItemProps,
MemoConversationListItemWithDetails,
} from './conversation-list-item/ConversationListItem';
import { useSelector } from 'react-redux';
import styled from 'styled-components';
@ -20,9 +16,11 @@ import { OverlayCommunity } from './overlay/OverlayCommunity';
import { OverlayMessage } from './overlay/OverlayMessage';
import { OverlayMessageRequest } from './overlay/OverlayMessageRequest';
import { OverlayChooseAction } from './overlay/choose-action/OverlayChooseAction';
import { ConversationListItem } from './conversation-list-item/ConversationListItem';
import { assertUnreachable } from '../../types/sqlSharedTypes';
export interface Props {
conversations?: Array<ConversationListItemProps>;
conversationIds?: Array<string>;
searchResults?: SearchResultsProps;
overlayMode: OverlayMode | undefined;
}
@ -57,8 +55,13 @@ const ClosableOverlay = () => {
return <OverlayMessage />;
case 'message-requests':
return <OverlayMessageRequest />;
default:
case undefined:
return null;
default:
return assertUnreachable(
overlayMode,
`ClosableOverlay: overlayMode case not handled "${overlayMode}"`
);
}
};
@ -69,38 +72,34 @@ export class LeftPaneMessageSection extends React.Component<Props> {
}
public renderRow = ({ index, key, style }: ListRowProps): JSX.Element | null => {
const { conversations } = this.props;
const { conversationIds } = this.props;
//assume conversations that have been marked unapproved should be filtered out by selector.
if (!conversations) {
if (!conversationIds) {
throw new Error('renderRow: Tried to render without conversations');
}
const conversation = conversations[index];
if (!conversation) {
const conversationId = conversationIds[index];
if (!conversationId) {
throw new Error('renderRow: conversations selector returned element containing falsy value.');
}
return <MemoConversationListItemWithDetails key={key} style={style} id={conversation.id} />; // TODO there should not be a need for the ...conversation here?
return <ConversationListItem key={key} style={style} conversationId={conversationId} />;
};
public renderList(): JSX.Element {
const { conversations, searchResults } = this.props;
const { conversationIds, searchResults } = this.props;
if (searchResults) {
return <SearchResults {...searchResults} />;
}
if (!conversations) {
if (!conversationIds) {
throw new Error('render: must provided conversations if no search results are provided');
}
const length = conversations.length;
const length = conversationIds.length;
// Note: conversations is not a known prop for List, but it is required to ensure that
// it re-renders when our conversations data changes. Otherwise it would just render
// on startup and scroll.
// TODO do need that `conversations` prop? I again don't see why it is needed. Especially because the list item use hook to fetch their details.
return (
<StyledLeftPaneList key={0}>
<AutoSizer>

View file

@ -6,10 +6,7 @@ import { Avatar, AvatarSize } from '../../avatar/Avatar';
import { createPortal } from 'react-dom';
import { useDispatch, useSelector } from 'react-redux';
import {
ReduxConversationType,
openConversationWithMessages,
} from '../../../state/ducks/conversations';
import { openConversationWithMessages } from '../../../state/ducks/conversations';
import { updateUserDetailsModal } from '../../../state/ducks/modalDialog';
import _, { isNil } from 'lodash';
@ -28,15 +25,12 @@ import { ContextConversationProvider, useConvoIdFromContext } from './ConvoIdCon
import { ConversationListItemHeaderItem } from './HeaderItem';
import { MessageItem } from './MessageItem';
// tslint:disable-next-line: no-empty-interface
export type ConversationListItemProps = Pick<ReduxConversationType, 'id'>;
type PropsHousekeeping = {
style?: Object;
};
// tslint:disable: use-simple-attributes
type Props = ConversationListItemProps & PropsHousekeeping;
type Props = { conversationId: string } & PropsHousekeeping;
const Portal = ({ children }: { children: any }) => {
return createPortal(children, document.querySelector('.inbox.index') as Element);
@ -70,8 +64,8 @@ const AvatarItem = () => {
);
};
const ConversationListItem = (props: Props) => {
const { id: conversationId, style } = props;
const ConversationListItemInner = (props: Props) => {
const { conversationId, style } = props;
const key = `conversation-item-${conversationId}`;
const hasUnread = useHasUnread(conversationId);
@ -140,4 +134,4 @@ const ConversationListItem = (props: Props) => {
);
};
export const MemoConversationListItemWithDetails = ConversationListItem;
export const ConversationListItem = ConversationListItemInner;

View file

@ -15,7 +15,7 @@ import { SpacerLG } from '../../basic/Text';
import useKey from 'react-use/lib/useKey';
import styled from 'styled-components';
import { SessionSearchInput } from '../../SessionSearchInput';
import { getSearchResults, isSearching } from '../../../state/selectors/search';
import { getSearchResultsContactOnly, isSearching } from '../../../state/selectors/search';
import { useSet } from '../../../hooks/useSet';
import { VALIDATION } from '../../../session/constants';
import { ToastUtils } from '../../../session/utils';
@ -96,6 +96,8 @@ export const OverlayClosedGroup = () => {
addTo: addToSelected,
removeFrom: removeFromSelected,
} = useSet<string>([]);
const isSearch = useSelector(isSearching);
const searchResultContactsOnly = useSelector(getSearchResultsContactOnly);
function closeOverlay() {
dispatch(resetOverlayMode());
@ -124,17 +126,7 @@ export const OverlayClosedGroup = () => {
const noContactsForClosedGroup = privateContactsPubkeys.length === 0;
const isSearch = useSelector(isSearching);
const searchResultsSelected = useSelector(getSearchResults);
const searchResults = isSearch ? searchResultsSelected : undefined;
let sharedWithResults: Array<string> = [];
if (searchResults && searchResults.contactsAndGroups.length) {
sharedWithResults = searchResults.contactsAndGroups
.filter(convo => convo.isPrivate)
.map(convo => convo.id);
}
const contactsToRender = isSearch ? sharedWithResults : privateContactsPubkeys;
const contactsToRender = isSearch ? searchResultContactsOnly : privateContactsPubkeys;
const disableCreateButton = !selectedMemberIds.length && !groupName.length;

View file

@ -8,11 +8,11 @@ import { declineConversationWithoutConfirm } from '../../../interactions/convers
import { forceSyncConfigurationNowIfNeeded } from '../../../session/utils/sync/syncUtils';
import { updateConfirmModal } from '../../../state/ducks/modalDialog';
import { resetOverlayMode } from '../../../state/ducks/section';
import { getConversationRequests } from '../../../state/selectors/conversations';
import { getConversationRequestsIds } from '../../../state/selectors/conversations';
import { useSelectedConversationKey } from '../../../state/selectors/selectedConversation';
import { SessionButton, SessionButtonColor } from '../../basic/SessionButton';
import { SpacerLG } from '../../basic/Text';
import { MemoConversationListItemWithDetails } from '../conversation-list-item/ConversationListItem';
import { ConversationListItem } from '../conversation-list-item/ConversationListItem';
const MessageRequestListPlaceholder = styled.div`
color: var(--conversation-tab-text-color);
@ -30,11 +30,11 @@ const MessageRequestListContainer = styled.div`
* @returns List of message request items
*/
const MessageRequestList = () => {
const conversationRequests = useSelector(getConversationRequests);
const conversationRequests = useSelector(getConversationRequestsIds);
return (
<MessageRequestListContainer>
{conversationRequests.map(conversation => {
return <MemoConversationListItemWithDetails key={conversation.id} {...conversation} />; // TODO there should not be a need for the ...conversation here?
{conversationRequests.map(conversationId => {
return <ConversationListItem key={conversationId} conversationId={conversationId} />;
})}
</MessageRequestListContainer>
);
@ -48,8 +48,8 @@ export const OverlayMessageRequest = () => {
}
const currentlySelectedConvo = useSelectedConversationKey();
const convoRequestCount = useSelector(getConversationRequests).length;
const messageRequests = useSelector(getConversationRequests);
const messageRequests = useSelector(getConversationRequestsIds);
const hasRequests = messageRequests.length;
const buttonText = window.i18n('clearAll');
@ -70,16 +70,16 @@ export const OverlayMessageRequest = () => {
onClose,
onClickOk: async () => {
window?.log?.info('Blocking all message requests');
if (!messageRequests) {
if (!hasRequests) {
window?.log?.info('No conversation requests to block.');
return;
}
for (let index = 0; index < messageRequests.length; index++) {
const convo = messageRequests[index];
const convoId = messageRequests[index];
await declineConversationWithoutConfirm({
blockContact: false,
conversationId: convo.id,
conversationId: convoId,
currentlySelectedConvo,
syncToDevices: false,
});
@ -93,7 +93,7 @@ export const OverlayMessageRequest = () => {
return (
<div className="module-left-pane-overlay">
{convoRequestCount ? (
{hasRequests ? (
<>
<MessageRequestList />
<SpacerLG />

View file

@ -1,13 +1,10 @@
import React from 'react';
import styled from 'styled-components';
import {
ConversationListItemProps,
MemoConversationListItemWithDetails,
} from '../leftpane/conversation-list-item/ConversationListItem';
import { ConversationListItem } from '../leftpane/conversation-list-item/ConversationListItem';
import { MessageResultProps, MessageSearchResult } from './MessageSearchResults';
export type SearchResultsProps = {
contactsAndGroups: Array<ConversationListItemProps>;
contactsAndGroupsIds: Array<string>;
messages: Array<MessageResultProps>;
searchTerm: string;
};
@ -39,9 +36,9 @@ const NoResults = styled.div`
`;
export const SearchResults = (props: SearchResultsProps) => {
const { contactsAndGroups, messages, searchTerm } = props;
const { contactsAndGroupsIds, messages, searchTerm } = props;
const haveContactsAndGroup = Boolean(contactsAndGroups?.length);
const haveContactsAndGroup = Boolean(contactsAndGroupsIds?.length);
const haveMessages = Boolean(messages?.length);
const noResults = !haveContactsAndGroup && !haveMessages;
@ -51,10 +48,10 @@ export const SearchResults = (props: SearchResultsProps) => {
{haveContactsAndGroup ? (
<>
<StyledSeparatorSection>{window.i18n('conversationsHeader')}</StyledSeparatorSection>
{contactsAndGroups.map(contactOrGroup => (
<MemoConversationListItemWithDetails
{...contactOrGroup}
key={`search-result-convo-${contactOrGroup.id}`}
{contactsAndGroupsIds.map(conversationId => (
<ConversationListItem
conversationId={conversationId}
key={`search-result-convo-${conversationId}`}
/>
))}
</>

View file

@ -1633,7 +1633,7 @@ function updateToSessionSchemaVersion31(currentVersion: number, db: BetterSqlite
{
url: ourDbProfileUrl,
key: ourDbProfileKey,
},
}
// ourConvoExpire
);
}

View file

@ -73,7 +73,6 @@ import {
} from './sqlInstance';
import { configDumpData } from './sql_calls/config_dump';
import { base64_variants, from_base64, to_hex } from 'libsodium-wrappers-sumo';
import { sleepFor } from '../session/utils/Promise';
// tslint:disable: no-console function-name non-literal-fs-path
@ -185,21 +184,16 @@ async function initializeSql({
// At this point we can allow general access to the database
initDbInstanceWith(db);
await sleepFor(10);
console.info('total message count before cleaning: ', getMessageCount());
console.info('total conversation count before cleaning: ', getConversationCount());
await sleepFor(10);
cleanUpOldOpengroupsOnStart();
cleanUpUnusedNodeForKeyEntriesOnStart();
await sleepFor(10);
printDbStats();
console.info('total message count after cleaning: ', getMessageCount());
console.info('total conversation count after cleaning: ', getConversationCount());
await sleepFor(10);
// Clear any already deleted db entries on each app start.
vacuumDatabase(db);
await sleepFor(10);
} catch (error) {
console.error('error', error);
if (passwordAttempt) {

View file

@ -245,41 +245,33 @@ export const _getConversationComparator = (testingi18n?: LocalizerType) => {
export const getConversationComparator = createSelector(getIntl, _getConversationComparator);
// tslint:disable-next-line: cyclomatic-complexity
const _getLeftPaneLists = (
const _getLeftPaneConversationIds = (
sortedConversations: Array<ReduxConversationType>
): Array<ReduxConversationType> => {
return sortedConversations.filter(conversation => {
if (conversation.isBlocked) {
return false;
}
): Array<string> => {
return sortedConversations
.filter(conversation => {
if (conversation.isBlocked) {
return false;
}
// a private conversation not approved is a message request. Exclude them from the left pane lists
// a non private conversation is always returned here
if (!conversation.isPrivate) {
return true;
}
if (conversation.isPrivate && !conversation.isApproved) {
return false;
}
// a private conversation not approved is a message request. Exclude them from the left pane lists
if (!conversation.isApproved) {
return false;
}
const isPrivateButHidden =
conversation.isPrivate &&
conversation.priority &&
conversation.priority <= CONVERSATION_PRIORITIES.default;
// a hidden contact conversation is only visible from the contact list, not from the global conversation list
if (conversation.priority && conversation.priority <= CONVERSATION_PRIORITIES.default) {
return false;
}
/**
* When getting a contact from a linked device, before he sent a message, the approved field is false, but a createdAt is used as activeAt
*/
const isPrivateUnapprovedButActive =
conversation.isPrivate && !conversation.isApproved && !conversation.activeAt;
if (
isPrivateUnapprovedButActive ||
isPrivateButHidden // a hidden contact conversation is only visible from the contact list, not from the global conversation list
) {
// dont increase unread counter, don't push to convo list.
return false;
}
return true;
});
return true;
})
.map(m => m.id);
};
// tslint:disable-next-line: cyclomatic-complexity
@ -307,25 +299,16 @@ const _getGlobalUnreadCount = (sortedConversations: Array<ReduxConversationType>
continue;
}
// a private conversation not approved is a message request. Exclude them from the left pane lists
// a private conversation not approved is a message request. Exclude them from the unread count
if (conversation.isPrivate && !conversation.isApproved) {
continue;
}
const isPrivateButHidden =
// a hidden contact conversation is only visible from the contact list, not from the global conversation list
if (
conversation.isPrivate &&
conversation.priority &&
conversation.priority <= CONVERSATION_PRIORITIES.default;
/**
* When getting a contact from a linked device, before he sent a message, the approved field is false, but a createdAt is used as activeAt
*/
const isPrivateUnapprovedButActive =
conversation.isPrivate && !conversation.isApproved && !conversation.activeAt;
if (
isPrivateUnapprovedButActive ||
isPrivateButHidden // a hidden contact conversation is only visible from the contact list, not from the global conversation list
conversation.priority <= CONVERSATION_PRIORITIES.default
) {
// dont increase unread counter, don't push to convo list.
continue;
@ -406,6 +389,14 @@ export const getConversationRequests = createSelector(
_getConversationRequests
);
export const getConversationRequestsIds = createSelector(getConversationRequests, requests =>
requests.map(m => m.id)
);
export const hasConversationRequests = (state: StateType) => {
return !!getConversationRequests(state).length;
};
const _getUnreadConversationRequests = (
sortedConversationRequests: Array<ReduxConversationType>
): Array<ReduxConversationType> => {
@ -427,15 +418,16 @@ export const getUnreadConversationRequests = createSelector(
* - approved (or message requests are disabled)
* - active_at is set to something truthy
*/
export const getPrivateContactsPubkeys = createSelector(_getPrivateFriendsConversations, state =>
state.map(m => m.id)
export const getLeftPaneConversationIds = createSelector(
getSortedConversations,
_getLeftPaneConversationIds
);
export const getLeftPaneLists = createSelector(getSortedConversations, _getLeftPaneLists);
const getDirectContacts = createSelector(getSortedConversations, _getPrivateFriendsConversations);
export const getDirectContacts = createSelector(
getSortedConversations,
_getPrivateFriendsConversations
export const getPrivateContactsPubkeys = createSelector(getDirectContacts, state =>
state.map(m => m.id)
);
export const getDirectContactsCount = createSelector(

View file

@ -15,7 +15,7 @@ export const isSearching = (state: StateType) => {
return !!getSearch(state)?.query?.trim();
};
export const getSearchResults = createSelector(
const getSearchResults = createSelector(
[getSearch, getConversationLookup],
(searchState: SearchStateType, lookup: ConversationLookupType) => {
return {
@ -40,3 +40,14 @@ export const getSearchResults = createSelector(
};
}
);
export const getSearchResultsIdsOnly = createSelector([getSearchResults], searchState => {
return {
...searchState,
contactsAndGroupsIds: searchState.contactsAndGroups.map(m => m.id),
};
});
export const getSearchResultsContactOnly = createSelector([getSearchResults], searchState => {
return searchState.contactsAndGroups.filter(m => m.isPrivate).map(m => m.id);
});