From 4e39f1e0ebe5a983e773f09e63e05ba0831b3b6e Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Mon, 30 Sep 2019 16:48:04 +1000 Subject: [PATCH 1/5] When composing: show mentions as profile names and treat them as indivisible elements --- js/views/conversation_view.js | 204 +++++++++++++++++++++++++++++++++- js/views/member_list_view.js | 42 +++++++ preload.js | 2 + 3 files changed, 243 insertions(+), 5 deletions(-) diff --git a/js/views/conversation_view.js b/js/views/conversation_view.js index 355d55617..359f89b4e 100644 --- a/js/views/conversation_view.js +++ b/js/views/conversation_view.js @@ -1603,11 +1603,16 @@ this.$messageField.val(), cursorPos ); - let firstHalf = `${prev}@${member.authorPhoneNumber}`; + + const handle = this.memberView.addPubkeyMapping( + member.authorProfileName, + member.authorPhoneNumber + ); + + let firstHalf = `${prev}${handle}`; let newCursorPos = firstHalf.length; - const needExtraWhitespace = - end.length === 0 || /[a-fA-F0-9@]/.test(end[0]); + const needExtraWhitespace = end.length === 0 || /\b/.test(end[0]); if (needExtraWhitespace) { firstHalf += ' '; newCursorPos += 1; @@ -1810,7 +1815,9 @@ this.model.clearTypingTimers(); const input = this.$messageField; - const message = window.Signal.Emoji.replaceColons(input.val()).trim(); + + let message = this.memberView.replaceMentions(input.val()); + message = window.Signal.Emoji.replaceColons(message).trim(); let toast; if (extension.expired()) { @@ -1853,6 +1860,7 @@ ); input.val(''); + this.memberView.deleteMention(); this.setQuoteMessage(null); this.resetLinkPreview(); this.focusMessageFieldAndClearDisabled(); @@ -2186,12 +2194,183 @@ } }, + handleDeleteOrBackspace(event, isDelete) { + const $input = this.$messageField[0]; + const text = this.$messageField.val(); + + // Only handle the case when nothing is selected + if ($input.selectionDirection !== 'none') { + // Note: if this ends up deleting a handle, we should + // (ideally) check if we need to update the mapping in + // `this.memberView`, but that's not vital as we already + // reset it on every 'send' + return; + } + + const mentions = this.memberView.pendingMentions(); + + const _ = window.Lodash; // no underscore.js please + const predicate = isDelete ? _.startsWith : _.endsWith; + + const pos = $input.selectionStart; + const part = isDelete ? text.substr(pos) : text.substr(0, pos); + + let curMention = null; + _.forEach(mentions, (val, key) => { + let shouldContinue = true; + if (predicate(part, key)) { + curMention = key; + shouldContinue = false; + } + return shouldContinue; + }); + + if (!curMention) { + return; + } + + event.preventDefault(); + + const beforeMention = isDelete + ? text.substr(0, pos) + : text.substr(0, pos - curMention.length); + const afterMention = isDelete + ? text.substr(pos + curMention.length) + : text.substr(pos); + + const resText = beforeMention + afterMention; + this.$messageField.val(resText); + $input.selectionStart = pos; + $input.selectionEnd = pos; + + this.memberView.deleteMention(curMention); + }, + + handleLeftRight(event, isLeft) { + // Return next cursor position candidate before we take + // various modifier keys into account + const nextPos = (text, cursorPos, isLeft2, isAltPressed) => { + // If the next char is ' ', skip it if Alt is pressed + let pos = cursorPos; + if (isAltPressed) { + const nextChar = isLeft2 + ? text.substr(pos - 1, 1) + : text.substr(pos, 1); + if (nextChar === ' ') { + pos = isLeft2 ? pos - 1 : pos + 1; + } + } + + const part = isLeft2 ? text.substr(0, pos) : text.substr(pos); + + const mentions = this.memberView.pendingMentions(); + + const predicate = isLeft2 + ? window.Lodash.endsWith + : window.Lodash.startsWith; + + let curMention = null; + _.forEach(mentions, (val, key) => { + let shouldContinue = true; + if (predicate(part, key)) { + curMention = key; + shouldContinue = false; + } + return shouldContinue; + }); + + const offset = curMention ? curMention.length : 1; + + const resPos = isLeft2 ? Math.max(0, pos - offset) : pos + offset; + + return resPos; + }; + + event.preventDefault(); + + const $input = this.$messageField[0]; + + const posStart = $input.selectionStart; + const posEnd = $input.selectionEnd; + + const text = this.$messageField.val(); + + const posToChange = + $input.selectionDirection === 'forward' ? posEnd : posStart; + + let newPos = nextPos(text, posToChange, isLeft, event.altKey); + + // If command (macos) key is pressed, go to the beginning/end + // (this shouldn't affect Windows, but we should double check that) + if (event.metaKey) { + newPos = isLeft ? 0 : text.length; + } + + // Alt would normally make the cursor go until the next whitespace, + // but we need to take the presence of a mention into account + if (event.altKey) { + const searchFrom = isLeft ? posToChange - 1 : posToChange + 1; + const toSearch = isLeft + ? text.substr(0, searchFrom) + : text.substr(searchFrom); + + // Note: we don't seem to support tabs etc, thus no /\s/ + let nextAltPos = isLeft + ? toSearch.lastIndexOf(' ') + : toSearch.indexOf(' '); + + if (nextAltPos === -1) { + nextAltPos = isLeft ? 0 : text.length; + } else if (isLeft) { + nextAltPos += 1; + } + + if (isLeft) { + newPos = Math.min(newPos, nextAltPos); + } else { + newPos = Math.max(newPos, nextAltPos + searchFrom); + } + } + + // ==== Handle selection business ==== + let newPosStart = newPos; + let newPosEnd = newPos; + + let direction = $input.selectionDirection; + + if (event.shiftKey) { + if (direction === 'none') { + if (isLeft) { + direction = 'backward'; + } else { + direction = 'forward'; + } + } + } else { + direction = 'none'; + } + + if (direction === 'forward') { + newPosStart = posStart; + } else if (direction === 'backward') { + newPosEnd = posEnd; + } + + if (newPosStart === newPosEnd) { + direction = 'none'; + } + + $input.setSelectionRange(newPosStart, newPosEnd, direction); + }, + // Note: not only input, but keypresses too (rename?) handleInputEvent(event) { // Note: schedule the member list handler shortly afterwards, so // that the input element has time to update its cursor position to // what the user would expect - window.requestAnimationFrame(this.maybeShowMembers.bind(this, event)); + if (this.model.isPublic()) { + window.requestAnimationFrame(this.maybeShowMembers.bind(this, event)); + } const keyCode = event.which || event.keyCode; @@ -2235,6 +2414,20 @@ if (keyPressedLeft || keyPressedRight) { this.$messageField.trigger('input'); + this.handleLeftRight(event, keyPressedLeft); + + return; + } + + const keyPressedDelete = keyCode === 46; + const keyPressedBackspace = keyCode === 8; + + if (keyPressedDelete) { + this.handleDeleteOrBackspace(event, true); + } + + if (keyPressedBackspace) { + this.handleDeleteOrBackspace(event, false); } this.updateMessageFieldSize(); @@ -2309,6 +2502,7 @@ let allMembers = window.lokiPublicChatAPI.getListOfMembers(); allMembers = allMembers.filter(d => !!d); + allMembers = allMembers.filter(d => d.authorProfileName !== 'Anonymous'); allMembers = _.uniq(allMembers, true, d => d.authorPhoneNumber); const cursorPos = event.target.selectionStart; diff --git a/js/views/member_list_view.js b/js/views/member_list_view.js index 33c3445d9..7c78c0a5c 100644 --- a/js/views/member_list_view.js +++ b/js/views/member_list_view.js @@ -9,6 +9,7 @@ Whisper.MemberListView = Whisper.View.extend({ initialize(options) { this.member_list = []; + this.memberMapping = {}; this.selected_idx = 0; this.onClicked = options.onClicked; this.render(); @@ -43,6 +44,31 @@ this.render(); } }, + replaceMentions(message) { + let result = message; + + // Sort keys from long to short, so we don't have to + // worry about one key being a substring of another + const keys = _.sortBy(_.keys(this.memberMapping), d => -d.length); + + keys.forEach(key => { + const pubkey = this.memberMapping[key]; + result = result.split(key).join(`@${pubkey}`); + }); + + return result; + }, + pendingMentions() { + return this.memberMapping; + }, + deleteMention(mention) { + if (mention) { + delete this.memberMapping[mention]; + } else { + // Delete all mentions if no argument is passed + this.memberMapping = {}; + } + }, membersShown() { return this.member_list.length !== 0; }, @@ -60,5 +86,21 @@ selectedMember() { return this.member_list[this.selected_idx]; }, + addPubkeyMapping(name, pubkey) { + let handle = `@${name}`; + let chars = 4; + + while ( + _.has(this.memberMapping, handle) && + this.memberMapping[handle] !== pubkey + ) { + const shortenedPubkey = pubkey.substr(pubkey.length - chars); + handle = `@${name}(..${shortenedPubkey})`; + chars += 1; + } + + this.memberMapping[handle] = pubkey; + return handle; + }, }); })(); diff --git a/preload.js b/preload.js index b3b462495..20df787e8 100644 --- a/preload.js +++ b/preload.js @@ -22,6 +22,8 @@ if (config.appInstance) { title += ` - ${config.appInstance}`; } +window.Lodash = require('lodash'); + window.platform = process.platform; window.getDefaultPoWDifficulty = () => config.defaultPoWDifficulty; window.getTitle = () => title; From 0980eafe7402cf4444e629402b86a0bb9e91017c Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Wed, 2 Oct 2019 10:45:33 +1000 Subject: [PATCH 2/5] First round of code review addressed --- js/views/conversation_view.js | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/js/views/conversation_view.js b/js/views/conversation_view.js index 359f89b4e..52bf769b9 100644 --- a/js/views/conversation_view.js +++ b/js/views/conversation_view.js @@ -2215,15 +2215,7 @@ const pos = $input.selectionStart; const part = isDelete ? text.substr(pos) : text.substr(0, pos); - let curMention = null; - _.forEach(mentions, (val, key) => { - let shouldContinue = true; - if (predicate(part, key)) { - curMention = key; - shouldContinue = false; - } - return shouldContinue; - }); + const curMention = _.keys(mentions).find(key => predicate(part, key)); if (!curMention) { return; @@ -2239,9 +2231,14 @@ : text.substr(pos); const resText = beforeMention + afterMention; + // NOTE: this doesn't work well with undo/redo, perhaps + // we should fix it one day this.$messageField.val(resText); - $input.selectionStart = pos; - $input.selectionEnd = pos; + + const nextPos = isDelete ? pos : pos - curMention.length; + + $input.selectionStart = nextPos; + $input.selectionEnd = nextPos; this.memberView.deleteMention(curMention); }, @@ -2269,15 +2266,7 @@ ? window.Lodash.endsWith : window.Lodash.startsWith; - let curMention = null; - _.forEach(mentions, (val, key) => { - let shouldContinue = true; - if (predicate(part, key)) { - curMention = key; - shouldContinue = false; - } - return shouldContinue; - }); + const curMention = _.keys(mentions).find(key => predicate(part, key)); const offset = curMention ? curMention.length : 1; From 7b0f40535f2eb6a403a47caaa276bf9590838a80 Mon Sep 17 00:00:00 2001 From: Beaudan Brown Date: Wed, 2 Oct 2019 10:31:26 +1000 Subject: [PATCH 3/5] QoL with auto focus display name box and enter/esc key functionality on profile screen. Restrict display name characters to alphanumeric (easy to work around) --- background.html | 2 +- js/views/standalone_registration_view.js | 46 ++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/background.html b/background.html index e3bbc00ba..ff532f26c 100644 --- a/background.html +++ b/background.html @@ -622,7 +622,7 @@
-
Enter a name that will be shown to all your contacts
+
Enter your public display name (alphanumeric characters and spaces only)
diff --git a/js/views/standalone_registration_view.js b/js/views/standalone_registration_view.js index 027bc0815..14f841e5f 100644 --- a/js/views/standalone_registration_view.js +++ b/js/views/standalone_registration_view.js @@ -1,4 +1,4 @@ -/* global Whisper, $, getAccountManager, textsecure, i18n, passwordUtil, _ */ +/* global Whisper, $, getAccountManager, textsecure, i18n, passwordUtil, _, setTimeout */ /* eslint-disable more/no-then */ @@ -8,6 +8,10 @@ window.Whisper = window.Whisper || {}; + const registerIndex = 0; + const profileIndex = 1; + let currentPageIndex = registerIndex; + Whisper.StandaloneRegistrationView = Whisper.View.extend({ templateName: 'standalone', className: 'full-screen-flow standalone-fullscreen', @@ -52,8 +56,25 @@ this.showRegisterPage(); this.onValidatePassword(); + + const sanitiseNameInput = () => { + const oldVal = this.$('#display-name').val(); + this.$('#display-name').val(oldVal.replace(/[^a-zA-Z0-9 ]/g, '')); + }; + + this.$('#display-name').get(0).oninput = () => { + sanitiseNameInput(); + }; + + this.$('#display-name').get(0).onpaste = () => { + // Sanitise data immediately after paste because it's easier + setTimeout(() => { + sanitiseNameInput(); + }); + }; }, events: { + keyup: 'onKeyup', 'validation input.number': 'onValidation', 'click #request-voice': 'requestVoice', 'click #request-sms': 'requestSMSVerification', @@ -77,12 +98,13 @@ $(this).hide(); } else { $(this).show(); + currentPageIndex = pageIndex; } }); }, async showRegisterPage() { this.registrationParams = {}; - this.showPage(0); + this.showPage(registerIndex); }, async showProfilePage(mnemonic, language) { this.registrationParams = { @@ -92,7 +114,25 @@ this.$passwordInput.val(''); this.$passwordConfirmationInput.val(''); this.onValidatePassword(); - this.showPage(1); + this.showPage(profileIndex); + this.$('#display-name').focus(); + }, + onKeyup(event) { + if (currentPageIndex !== profileIndex) { + // Only want enter/escape keys to work on profile page + return; + } + + switch (event.key) { + case 'Enter': + this.onSaveProfile(); + break; + case 'Escape': + case 'Esc': + this.onBack(); + break; + default: + } }, async register(mnemonic, language) { // Make sure the password is valid From 64ccd05a2ec2869fe10f6f82cd0a39742b1e632b Mon Sep 17 00:00:00 2001 From: Beaudan Brown Date: Wed, 2 Oct 2019 12:06:57 +1000 Subject: [PATCH 4/5] Capitalise constants --- js/views/standalone_registration_view.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/js/views/standalone_registration_view.js b/js/views/standalone_registration_view.js index 14f841e5f..edf799850 100644 --- a/js/views/standalone_registration_view.js +++ b/js/views/standalone_registration_view.js @@ -8,9 +8,9 @@ window.Whisper = window.Whisper || {}; - const registerIndex = 0; - const profileIndex = 1; - let currentPageIndex = registerIndex; + const REGISTER_INDEX = 0; + const PROFILE_INDEX = 1; + let currentPageIndex = REGISTER_INDEX; Whisper.StandaloneRegistrationView = Whisper.View.extend({ templateName: 'standalone', @@ -104,7 +104,7 @@ }, async showRegisterPage() { this.registrationParams = {}; - this.showPage(registerIndex); + this.showPage(REGISTER_INDEX); }, async showProfilePage(mnemonic, language) { this.registrationParams = { @@ -114,11 +114,11 @@ this.$passwordInput.val(''); this.$passwordConfirmationInput.val(''); this.onValidatePassword(); - this.showPage(profileIndex); + this.showPage(PROFILE_INDEX); this.$('#display-name').focus(); }, onKeyup(event) { - if (currentPageIndex !== profileIndex) { + if (currentPageIndex !== PROFILE_INDEX) { // Only want enter/escape keys to work on profile page return; } From 8be1c61f4c577289b6ff95f2b582bb152b352ffa Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Wed, 2 Oct 2019 13:28:10 +1000 Subject: [PATCH 5/5] Highlight conversations with unread mentions of the user --- js/models/conversations.js | 16 ++++++++++ js/models/messages.js | 6 ++++ js/views/conversation_view.js | 3 +- stylesheets/_mentions.scss | 29 +++++++++++++++++++ stylesheets/_modules.scss | 2 +- ts/components/ConversationListItem.tsx | 16 ++++++++-- ts/state/ducks/conversations.ts | 1 + ts/test/state/selectors/conversations_test.ts | 5 ++++ 8 files changed, 73 insertions(+), 5 deletions(-) diff --git a/js/models/conversations.js b/js/models/conversations.js index f03eb4a73..de9c2216f 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -466,6 +466,7 @@ timestamp: this.get('timestamp'), title: this.getTitle(), unreadCount: this.get('unreadCount') || 0, + mentionedUs: this.get('mentionedUs') || false, showFriendRequestIndicator: this.isPendingFriendRequest(), isBlocked: this.isBlocked(), @@ -2007,6 +2008,21 @@ const unreadCount = unreadMessages.length - read.length; this.set({ unreadCount }); + + const mentionRead = (() => { + const stillUnread = unreadMessages.filter( + m => m.get('received_at') > newestUnreadDate + ); + const ourNumber = textsecure.storage.user.getNumber(); + return !stillUnread.some( + m => m.propsForMessage.text.indexOf(`@${ourNumber}`) !== -1 + ); + })(); + + if (mentionRead) { + this.set({ mentionedUs: false }); + } + await window.Signal.Data.updateConversation(this.id, this.attributes, { Conversation: Whisper.Conversation, }); diff --git a/js/models/messages.js b/js/models/messages.js index c0301ec05..7c2022fe6 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -1969,6 +1969,12 @@ c.onReadMessage(message); } } else { + const ourNumber = textsecure.storage.user.getNumber(); + + if (message.attributes.body.indexOf(`@${ourNumber}`) !== -1) { + conversation.set({ mentionedUs: true }); + } + conversation.set({ unreadCount: conversation.get('unreadCount') + 1, isArchived: false, diff --git a/js/views/conversation_view.js b/js/views/conversation_view.js index 355d55617..6606343fd 100644 --- a/js/views/conversation_view.js +++ b/js/views/conversation_view.js @@ -314,10 +314,11 @@ this.selectMember = this.selectMember.bind(this); const updateMemberList = async () => { + const maxToFetch = 1000; const allMessages = await window.Signal.Data.getMessagesByConversation( this.model.id, { - limit: Number.MAX_SAFE_INTEGER, + limit: maxToFetch, MessageCollection: Whisper.MessageCollection, } ); diff --git a/stylesheets/_mentions.scss b/stylesheets/_mentions.scss index 58c65c5af..150e94237 100644 --- a/stylesheets/_mentions.scss +++ b/stylesheets/_mentions.scss @@ -67,3 +67,32 @@ } } } + +.module-conversation-list-item--mentioned-us { + border-left: 4px solid #ffb000 !important; +} + +.at-symbol { + background-color: #ffb000; + + color: $color-black; + text-align: center; + + padding-top: 1px; + padding-left: 3px; + padding-right: 3px; + + position: absolute; + right: -6px; + top: 12px; + + font-weight: 300; + font-size: 11px; + letter-spacing: 0.25px; + + height: 16px; + min-width: 16px; + border-radius: 8px; + + box-shadow: 0px 0px 0px 1px $color-dark-85; +} diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index b109feb2d..87e8e5e45 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -1895,7 +1895,7 @@ position: absolute; right: -6px; - top: 6px; + top: -6px; font-weight: 300; font-size: 11px; diff --git a/ts/components/ConversationListItem.tsx b/ts/components/ConversationListItem.tsx index 3a2fab947..8a973420d 100644 --- a/ts/components/ConversationListItem.tsx +++ b/ts/components/ConversationListItem.tsx @@ -26,6 +26,7 @@ export type PropsData = { lastUpdated: number; unreadCount: number; + mentionedUs: boolean; isSelected: boolean; isTyping: boolean; @@ -93,12 +94,17 @@ export class ConversationListItem extends React.PureComponent { } public renderUnread() { - const { unreadCount } = this.props; + const { unreadCount, mentionedUs } = this.props; if (unreadCount > 0) { + const atSymbol = mentionedUs ?

@

: null; + return ( -
- {unreadCount} +
+

+ {unreadCount} +

+ {atSymbol}
); } @@ -285,6 +291,7 @@ export class ConversationListItem extends React.PureComponent { showFriendRequestIndicator, isBlocked, style, + mentionedUs, } = this.props; const triggerId = `${phoneNumber}-ctxmenu-${Date.now()}`; @@ -305,6 +312,9 @@ export class ConversationListItem extends React.PureComponent { unreadCount > 0 ? 'module-conversation-list-item--has-unread' : null, + unreadCount > 0 && mentionedUs + ? 'module-conversation-list-item--mentioned-us' + : null, isSelected ? 'module-conversation-list-item--is-selected' : null, showFriendRequestIndicator ? 'module-conversation-list-item--has-friend-request' diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index b0ced5094..ab189d13c 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -49,6 +49,7 @@ export type ConversationType = { isClosable?: boolean; lastUpdated: number; unreadCount: number; + mentionedUs: boolean; isSelected: boolean; isTyping: boolean; isFriend?: boolean; diff --git a/ts/test/state/selectors/conversations_test.ts b/ts/test/state/selectors/conversations_test.ts index c56f50f2a..63b6cd9e9 100644 --- a/ts/test/state/selectors/conversations_test.ts +++ b/ts/test/state/selectors/conversations_test.ts @@ -24,6 +24,7 @@ describe('state/selectors/conversations', () => { isMe: false, lastUpdated: Date.now(), unreadCount: 1, + mentionedUs: false, isSelected: false, isTyping: false, }, @@ -39,6 +40,7 @@ describe('state/selectors/conversations', () => { isMe: false, lastUpdated: Date.now(), unreadCount: 1, + mentionedUs: false, isSelected: false, isTyping: false, }, @@ -54,6 +56,7 @@ describe('state/selectors/conversations', () => { isMe: false, lastUpdated: Date.now(), unreadCount: 1, + mentionedUs: false, isSelected: false, isTyping: false, }, @@ -69,6 +72,7 @@ describe('state/selectors/conversations', () => { isMe: false, lastUpdated: Date.now(), unreadCount: 1, + mentionedUs: false, isSelected: false, isTyping: false, }, @@ -84,6 +88,7 @@ describe('state/selectors/conversations', () => { isMe: false, lastUpdated: Date.now(), unreadCount: 1, + mentionedUs: false, isSelected: false, isTyping: false, },