Responding to feedback on the updated visuals ()

* Conversation List Item: timestamp bold only when convo has unread

* Preserve the positioning of overlays on re-entry into convo

* ConversationListItem: Handle missing and broken thumbnails

* Shorten timestamp in left pane for better Android consistency

* Update convo last updated if last was expire timer change

But not if it was from a sync instead of from you or from a contact.

* Make links in quotes the same color as the text

* MediaGridItem: Update placeholder icon colors for dark theme

* Ensure turning off timer shows 'Timer set to off' in left pane

* ConversationListItem: Show unread count in blue circle

* Add one pixel margin to blue indicator for text alignment

* Ensure replies to voice message can bet sent successfully
This commit is contained in:
Scott Nonnenberg 2018-07-20 16:37:57 -07:00 committed by GitHub
parent 60d56cf7e0
commit 643739f65d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 348 additions and 38 deletions

View file

@ -1149,6 +1149,17 @@
"description": "description":
"Brief timestamp for messages sent about one hour ago. Displayed in the conversation list and message bubble." "Brief timestamp for messages sent about one hour ago. Displayed in the conversation list and message bubble."
}, },
"hoursAgoShort": {
"message": "$hours$ hr",
"description":
"Even further contracted form of 'X hours ago' which works both for singular and plural, used in the left pane",
"placeholders": {
"hours": {
"content": "$1",
"example": "2"
}
}
},
"hoursAgo": { "hoursAgo": {
"message": "$hours$ hr ago", "message": "$hours$ hr ago",
"description": "description":
@ -1160,6 +1171,17 @@
} }
} }
}, },
"minutesAgoShort": {
"message": "$minutes$ min",
"description":
"Even further contracted form of 'X minutes ago' which works both for singular and plural, used in the left pane",
"placeholders": {
"minutes": {
"content": "$1",
"example": "10"
}
}
},
"minutesAgo": { "minutesAgo": {
"message": "$minutes$ min ago", "message": "$minutes$ min ago",
"description": "description":

View file

@ -207,7 +207,7 @@
...this.format(), ...this.format(),
lastUpdated: this.get('timestamp'), lastUpdated: this.get('timestamp'),
hasUnread: Boolean(this.get('unreadCount')), unreadCount: this.get('unreadCount') || 0,
isSelected: this.isSelected, isSelected: this.isSelected,
lastMessage: { lastMessage: {
@ -795,7 +795,9 @@
return { return {
contentType, contentType,
fileName, // Our protos library complains about this field being undefined, so we
// force it to null
fileName: fileName || null,
thumbnail: thumbnail thumbnail: thumbnail
? { ? {
...(await loadAttachmentData(thumbnail)), ...(await loadAttachmentData(thumbnail)),

View file

@ -193,7 +193,7 @@
const { expireTimer } = this.get('expirationTimerUpdate'); const { expireTimer } = this.get('expirationTimerUpdate');
return i18n( return i18n(
'timerSetTo', 'timerSetTo',
Whisper.ExpirationTimerOptions.getAbbreviated(expireTimer) Whisper.ExpirationTimerOptions.getAbbreviated(expireTimer || 0)
); );
} }
if (this.isKeyChange()) { if (this.isKeyChange()) {

View file

@ -752,6 +752,10 @@
}, },
focusMessageField() { focusMessageField() {
if (this.panels && this.panels.length) {
return;
}
this.$messageField.focus(); this.$messageField.focus();
}, },
@ -1286,6 +1290,7 @@
if (message) { if (message) {
const quote = await this.model.makeQuote(this.quotedMessage); const quote = await this.model.makeQuote(this.quotedMessage);
console.log('DEBUG', { quote });
this.quote = quote; this.quote = quote;
this.focusMessageFieldAndClearDisabled(); this.focusMessageFieldAndClearDisabled();

View file

@ -810,6 +810,10 @@
line-height: 18px; line-height: 18px;
color: $color-light-90; color: $color-light-90;
a {
color: $color-light-90;
}
overflow-wrap: break-word; overflow-wrap: break-word;
word-wrap: break-word; word-wrap: break-word;
word-break: break-word; word-break: break-word;
@ -1805,6 +1809,7 @@
background-color: $color-light-10; background-color: $color-light-10;
margin-right: 4px; margin-right: 4px;
margin-bottom: 4px; margin-bottom: 4px;
position: relative;
} }
.module-media-grid-item__image { .module-media-grid-item__image {
@ -1813,6 +1818,18 @@
object-fit: cover; object-fit: cover;
} }
.module-media-grid-item__icon {
position: absolute;
top: 15px;
bottom: 15px;
left: 15px;
right: 15px;
}
.module-media-grid-item__icon-image {
@include color-svg('../images/image.svg', $color-light-35);
}
.module-media-grid-item__image-container { .module-media-grid-item__image-container {
height: 94px; height: 94px;
width: 94px; width: 94px;
@ -1844,6 +1861,14 @@
@include color-svg('../images/play.svg', $color-signal-blue); @include color-svg('../images/play.svg', $color-signal-blue);
} }
.module-media-grid-item__icon-video {
@include color-svg('../images/movie.svg', $color-light-35);
}
.module-media-grid-item__icon-generic {
@include color-svg('../images/file.svg', $color-light-35);
}
/* Module: Empty State*/ /* Module: Empty State*/
.module-empty-state { .module-empty-state {
@ -1964,7 +1989,6 @@
font-size: 11px; font-size: 11px;
line-height: 16px; line-height: 16px;
letter-spacing: 0.3px; letter-spacing: 0.3px;
font-weight: 300;
overflow-x: hidden; overflow-x: hidden;
white-space: nowrap; white-space: nowrap;
@ -1973,17 +1997,22 @@
text-transform: uppercase; text-transform: uppercase;
} }
.module-conversation-list-item__header__date--has-unread {
font-weight: 300;
}
.module-conversation-list-item__message { .module-conversation-list-item__message {
display: flex; display: flex;
flex-direction: row; flex-direction: row;
align-items: center; align-items: center;
margin-top: 3px;
} }
.module-conversation-list-item__message__text { .module-conversation-list-item__message__text {
flex-grow: 1; flex-grow: 1;
flex-shrink: 1; flex-shrink: 1;
margin-top: 3px;
font-size: 13px; font-size: 13px;
line-height: 18px; line-height: 18px;
@ -1997,6 +2026,23 @@
font-weight: 300; font-weight: 300;
} }
.module-conversation-list-item__unread-count {
color: $color-white;
background-color: $color-signal-blue;
text-align: center;
// For alignment with the message text
margin-top: 1px;
font-size: 10px;
margin-left: 5px;
min-width: 20px;
height: 20px;
width: 20px;
line-height: 20px;
border-radius: 10px;
}
.module-conversation-list-item__message__status-icon { .module-conversation-list-item__message__status-icon {
flex-shrink: 0; flex-shrink: 0;

View file

@ -927,6 +927,10 @@ body.dark-theme {
.module-quote__primary__text { .module-quote__primary__text {
color: $color-dark-05; color: $color-dark-05;
a {
color: $color-dark-05;
}
} }
.module-quote__primary__type-label { .module-quote__primary__type-label {
@ -1275,6 +1279,18 @@ body.dark-theme {
background-color: $color-dark-85; background-color: $color-dark-85;
} }
.module-media-grid-item__icon-image {
@include color-svg('../images/image.svg', $color-dark-60);
}
.module-media-grid-item__icon-video {
@include color-svg('../images/movie.svg', $color-dark-60);
}
.module-media-grid-item__icon-generic {
@include color-svg('../images/file.svg', $color-dark-60);
}
// Module: Empty State // Module: Empty State
.module-empty-state { .module-empty-state {

View file

@ -22,6 +22,7 @@
phoneNumber="(202) 555-0011" phoneNumber="(202) 555-0011"
name="Mr. Fire🔥" name="Mr. Fire🔥"
color="green" color="green"
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{ lastMessage={{
text: 'Just a second', text: 'Just a second',
status: 'read', status: 'read',
@ -34,16 +35,38 @@
#### With unread #### With unread
```jsx ```jsx
<ConversationListItem <div>
phoneNumber="(202) 555-0011" <ConversationListItem
hasUnread={true} phoneNumber="(202) 555-0011"
lastMessage={{ unreadCount={4}
text: 'Hey there!', lastUpdated={Date.now() - 5 * 60 * 1000}
status: 'sending', lastMessage={{
}} text: 'Hey there!',
onClick={() => console.log('onClick')} }}
i18n={util.i18n} onClick={() => console.log('onClick')}
/> i18n={util.i18n}
/>
<ConversationListItem
phoneNumber="(202) 555-0011"
unreadCount={10}
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{
text: 'Hey there!',
}}
onClick={() => console.log('onClick')}
i18n={util.i18n}
/>
<ConversationListItem
phoneNumber="(202) 555-0011"
unreadCount={250}
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{
text: 'Hey there!',
}}
onClick={() => console.log('onClick')}
i18n={util.i18n}
/>
</div>
``` ```
#### Selected #### Selected
@ -52,6 +75,7 @@
<ConversationListItem <ConversationListItem
phoneNumber="(202) 555-0011" phoneNumber="(202) 555-0011"
isSelected={true} isSelected={true}
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{ lastMessage={{
text: 'Hey there!', text: 'Hey there!',
}} }}
@ -68,6 +92,7 @@ We don't want Jumbomoji or links.
<div> <div>
<ConversationListItem <ConversationListItem
phoneNumber="(202) 555-0011" phoneNumber="(202) 555-0011"
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{ lastMessage={{
text: 'Download at http://signal.org', text: 'Download at http://signal.org',
}} }}
@ -76,6 +101,7 @@ We don't want Jumbomoji or links.
/> />
<ConversationListItem <ConversationListItem
phoneNumber="(202) 555-0011" phoneNumber="(202) 555-0011"
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{ lastMessage={{
text: '🔥', text: '🔥',
}} }}
@ -94,6 +120,7 @@ We only show one line.
<ConversationListItem <ConversationListItem
phoneNumber="(202) 555-0011" phoneNumber="(202) 555-0011"
name="Long contact name. Esquire. The third. And stuff. And more! And more!" name="Long contact name. Esquire. The third. And stuff. And more! And more!"
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{ lastMessage={{
text: 'Normal message', text: 'Normal message',
}} }}
@ -102,6 +129,7 @@ We only show one line.
/> />
<ConversationListItem <ConversationListItem
phoneNumber="(202) 555-0011" phoneNumber="(202) 555-0011"
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{ lastMessage={{
text: text:
"Long line. This is a really really really long line. Really really long. Because that's just how it is", "Long line. This is a really really really long line. Really really long. Because that's just how it is",
@ -111,6 +139,7 @@ We only show one line.
/> />
<ConversationListItem <ConversationListItem
phoneNumber="(202) 555-0011" phoneNumber="(202) 555-0011"
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{ lastMessage={{
text: text:
"Long line. This is a really really really long line. Really really long. Because that's just how it is", "Long line. This is a really really really long line. Really really long. Because that's just how it is",
@ -122,6 +151,18 @@ We only show one line.
<ConversationListItem <ConversationListItem
phoneNumber="(202) 555-0011" phoneNumber="(202) 555-0011"
lastUpdated={Date.now() - 5 * 60 * 1000}
unreadCount={8}
lastMessage={{
text:
"Long line. This is a really really really long line. Really really long. Because that's just how it is",
}}
onClick={() => console.log('onClick')}
i18n={util.i18n}
/>
<ConversationListItem
phoneNumber="(202) 555-0011"
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{ lastMessage={{
text: text:
"Many lines. This is a many-line message.\nLine 2 is really exciting but it shouldn't be seen.\nLine three is even better.\nLine 4, well.", "Many lines. This is a many-line message.\nLine 2 is really exciting but it shouldn't be seen.\nLine three is even better.\nLine 4, well.",
@ -131,6 +172,7 @@ We only show one line.
/> />
<ConversationListItem <ConversationListItem
phoneNumber="(202) 555-0011" phoneNumber="(202) 555-0011"
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{ lastMessage={{
text: text:
"Many lines. This is a many-line message.\nLine 2 is really exciting but it shouldn't be seen.\nLine three is even better.\nLine 4, well.", "Many lines. This is a many-line message.\nLine 2 is really exciting but it shouldn't be seen.\nLine three is even better.\nLine 4, well.",
@ -142,6 +184,35 @@ We only show one line.
</div> </div>
``` ```
#### More narrow
On platforms that show scrollbars all the time, this is true all the time.
```jsx
<div style={{ width: '280px' }}>
<ConversationListItem
phoneNumber="(202) 555-0011"
name="Long contact name. Esquire. The third. And stuff. And more! And more!"
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{
text: 'Normal message',
}}
onClick={() => console.log('onClick')}
i18n={util.i18n}
/>
<ConversationListItem
phoneNumber="(202) 555-0011"
lastUpdated={Date.now() - 5 * 60 * 1000}
lastMessage={{
text:
"Long line. This is a really really really long line. Really really long. Because that's just how it is",
}}
onClick={() => console.log('onClick')}
i18n={util.i18n}
/>
</div>
```
#### With various ages #### With various ages
```jsx ```jsx

View file

@ -14,7 +14,7 @@ interface Props {
avatarPath?: string; avatarPath?: string;
lastUpdated: number; lastUpdated: number;
hasUnread: boolean; unreadCount: number;
isSelected: boolean; isSelected: boolean;
lastMessage?: { lastMessage?: {
@ -71,7 +71,14 @@ export class ConversationListItem extends React.Component<Props> {
} }
public renderHeader() { public renderHeader() {
const { i18n, lastUpdated, name, phoneNumber, profileName } = this.props; const {
unreadCount,
i18n,
lastUpdated,
name,
phoneNumber,
profileName,
} = this.props;
return ( return (
<div className="module-conversation-list-item__header"> <div className="module-conversation-list-item__header">
@ -83,7 +90,14 @@ export class ConversationListItem extends React.Component<Props> {
i18n={i18n} i18n={i18n}
/> />
</div> </div>
<div className="module-conversation-list-item__header__date"> <div
className={classNames(
'module-conversation-list-item__header__date',
unreadCount > 0
? 'module-conversation-list-item__header__date--has-unread'
: null
)}
>
<Timestamp <Timestamp
timestamp={lastUpdated} timestamp={lastUpdated}
extended={false} extended={false}
@ -95,8 +109,22 @@ export class ConversationListItem extends React.Component<Props> {
); );
} }
public renderUnread() {
const { unreadCount } = this.props;
if (unreadCount > 0) {
return (
<div className="module-conversation-list-item__unread-count">
{unreadCount}
</div>
);
}
return null;
}
public renderMessage() { public renderMessage() {
const { lastMessage, hasUnread, i18n } = this.props; const { lastMessage, unreadCount, i18n } = this.props;
if (!lastMessage) { if (!lastMessage) {
return null; return null;
@ -108,7 +136,7 @@ export class ConversationListItem extends React.Component<Props> {
<div <div
className={classNames( className={classNames(
'module-conversation-list-item__message__text', 'module-conversation-list-item__message__text',
hasUnread unreadCount > 0
? 'module-conversation-list-item__message__text--has-unread' ? 'module-conversation-list-item__message__text--has-unread'
: null : null
)} )}
@ -131,12 +159,13 @@ export class ConversationListItem extends React.Component<Props> {
)} )}
/> />
) : null} ) : null}
{this.renderUnread()}
</div> </div>
); );
} }
public render() { public render() {
const { hasUnread, onClick, isSelected } = this.props; const { unreadCount, onClick, isSelected } = this.props;
return ( return (
<div <div
@ -144,7 +173,7 @@ export class ConversationListItem extends React.Component<Props> {
onClick={onClick} onClick={onClick}
className={classNames( className={classNames(
'module-conversation-list-item', 'module-conversation-list-item',
hasUnread ? 'module-conversation-list-item--has-unread' : null, unreadCount > 0 ? 'module-conversation-list-item--has-unread' : null,
isSelected ? 'module-conversation-list-item--is-selected' : null isSelected ? 'module-conversation-list-item--is-selected' : null
)} )}
> >

View file

@ -1,4 +1,4 @@
## With image #### With image
```jsx ```jsx
const message = { const message = {
@ -14,7 +14,7 @@ const message = {
<MediaGridItem i18n={util.i18n} message={message} />; <MediaGridItem i18n={util.i18n} message={message} />;
``` ```
## With video #### With video
```jsx ```jsx
const message = { const message = {
@ -30,7 +30,69 @@ const message = {
<MediaGridItem i18n={util.i18n} message={message} />; <MediaGridItem i18n={util.i18n} message={message} />;
``` ```
## Without image #### Missing image
```jsx
const message = {
id: '1',
attachments: [
{
fileName: 'foo.jpg',
contentType: 'image/jpeg',
},
],
};
<MediaGridItem i18n={util.i18n} message={message} />;
```
#### Missing video
```jsx
const message = {
id: '1',
attachments: [
{
fileName: 'foo.jpg',
contentType: 'video/mp4',
},
],
};
<MediaGridItem i18n={util.i18n} message={message} />;
```
#### Image thumbnail failed to load
```jsx
const message = {
id: '1',
thumbnailObjectUrl: 'nonexistent',
attachments: [
{
fileName: 'foo.jpg',
contentType: 'image/jpeg',
},
],
};
<MediaGridItem i18n={util.i18n} message={message} />;
```
#### Video thumbnail failed to load
```jsx
const message = {
id: '1',
thumbnailObjectUrl: 'nonexistent',
attachments: [
{
fileName: 'foo.jpg',
contentType: 'video/mp4',
},
],
};
<MediaGridItem i18n={util.i18n} message={message} />;
```
#### Other contentType
```jsx ```jsx
const message = { const message = {

View file

@ -1,4 +1,5 @@
import React from 'react'; import React from 'react';
import classNames from 'classnames';
import { import {
isImageTypeSupported, isImageTypeSupported,
@ -13,14 +14,34 @@ interface Props {
i18n: Localizer; i18n: Localizer;
} }
export class MediaGridItem extends React.Component<Props> { interface State {
imageBroken: boolean;
}
export class MediaGridItem extends React.Component<Props, State> {
private onImageErrorBound: () => void;
constructor(props: Props) {
super(props);
this.state = {
imageBroken: false,
};
this.onImageErrorBound = this.onImageError.bind(this);
}
public onImageError() {
this.setState({
imageBroken: true,
});
}
public renderContent() { public renderContent() {
const { message, i18n } = this.props; const { message, i18n } = this.props;
const { imageBroken } = this.state;
const { attachments } = message; const { attachments } = message;
if (!message.thumbnailObjectUrl) {
return null;
}
if (!attachments || !attachments.length) { if (!attachments || !attachments.length) {
return null; return null;
} }
@ -29,20 +50,44 @@ export class MediaGridItem extends React.Component<Props> {
const { contentType } = first; const { contentType } = first;
if (contentType && isImageTypeSupported(contentType)) { if (contentType && isImageTypeSupported(contentType)) {
if (imageBroken || !message.thumbnailObjectUrl) {
return (
<div
className={classNames(
'module-media-grid-item__icon',
'module-media-grid-item__icon-image'
)}
/>
);
}
return ( return (
<img <img
alt={i18n('lightboxImageAlt')} alt={i18n('lightboxImageAlt')}
className="module-media-grid-item__image" className="module-media-grid-item__image"
src={message.thumbnailObjectUrl} src={message.thumbnailObjectUrl}
onError={this.onImageErrorBound}
/> />
); );
} else if (contentType && isVideoTypeSupported(contentType)) { } else if (contentType && isVideoTypeSupported(contentType)) {
if (imageBroken || !message.thumbnailObjectUrl) {
return (
<div
className={classNames(
'module-media-grid-item__icon',
'module-media-grid-item__icon-video'
)}
/>
);
}
return ( return (
<div className="module-media-grid-item__image-container"> <div className="module-media-grid-item__image-container">
<img <img
alt={i18n('lightboxImageAlt')} alt={i18n('lightboxImageAlt')}
className="module-media-grid-item__image" className="module-media-grid-item__image"
src={message.thumbnailObjectUrl} src={message.thumbnailObjectUrl}
onError={this.onImageErrorBound}
/> />
<div className="module-media-grid-item__circle-overlay"> <div className="module-media-grid-item__circle-overlay">
<div className="module-media-grid-item__play-overlay" /> <div className="module-media-grid-item__play-overlay" />
@ -51,7 +96,14 @@ export class MediaGridItem extends React.Component<Props> {
); );
} }
return null; return (
<div
className={classNames(
'module-media-grid-item__icon',
'module-media-grid-item__icon-generic'
)}
/>
);
} }
public render() { public render() {

View file

@ -76,7 +76,7 @@ describe('Conversation', () => {
}); });
}); });
context('for expired message', () => { context('for expire timer update from sync', () => {
it('should update message but not timestamp (to prevent bump to top)', () => { it('should update message but not timestamp (to prevent bump to top)', () => {
const input = { const input = {
currentLastMessageText: 'I am expired', currentLastMessageText: 'I am expired',
@ -89,7 +89,7 @@ describe('Conversation', () => {
timestamp: 666, timestamp: 666,
expirationTimerUpdate: { expirationTimerUpdate: {
expireTimer: 111, expireTimer: 111,
fromSync: false, fromSync: true,
source: '+12223334455', source: '+12223334455',
}, },
} as IncomingMessage, } as IncomingMessage,

View file

@ -1,4 +1,3 @@
import is from '@sindresorhus/is';
import { Message } from './Message'; import { Message } from './Message';
interface ConversationLastMessageUpdate { interface ConversationLastMessageUpdate {
@ -28,10 +27,12 @@ export const createLastMessageUpdate = ({
}; };
} }
const { type } = lastMessage; const { type, expirationTimerUpdate } = lastMessage;
const isVerifiedChangeMessage = type === 'verified-change'; const isVerifiedChangeMessage = type === 'verified-change';
const isExpiringMessage = is.object(lastMessage.expirationTimerUpdate); const isExpireTimerUpdateFromSync =
const shouldUpdateTimestamp = !isVerifiedChangeMessage && !isExpiringMessage; expirationTimerUpdate && expirationTimerUpdate.fromSync;
const shouldUpdateTimestamp =
!isVerifiedChangeMessage && !isExpireTimerUpdateFromSync;
const newTimestamp = shouldUpdateTimestamp const newTimestamp = shouldUpdateTimestamp
? lastMessage.sent_at ? lastMessage.sent_at

View file

@ -44,9 +44,13 @@ export function formatRelativeTime(
} else if (diff.days() >= 1 || !isToday(timestamp)) { } else if (diff.days() >= 1 || !isToday(timestamp)) {
return timestamp.format(formats.d); return timestamp.format(formats.d);
} else if (diff.hours() >= 1) { } else if (diff.hours() >= 1) {
return i18n('hoursAgo', [String(diff.hours())]); const key = extended ? 'hoursAgo' : 'hoursAgoShort';
return i18n(key, [String(diff.hours())]);
} else if (diff.minutes() >= 1) { } else if (diff.minutes() >= 1) {
return i18n('minutesAgo', [String(diff.minutes())]); const key = extended ? 'minutesAgo' : 'minutesAgoShort';
return i18n(key, [String(diff.minutes())]);
} }
return i18n('justNow'); return i18n('justNow');