Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve UI performance of ephemeral messages #11071

Merged
merged 2 commits into from May 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/page/template/modal/user-modal.htm
Expand Up @@ -17,7 +17,7 @@
params="user: user, actionsViewModel: actionsViewModel, onAction: onUserAction, isSelfActivated: isActivatedAccount()"
></user-actions>
<!-- /ko -->
<!-- ko if: !user() && !userNotFound() -->
<!-- ko if: isVisible() && !user() && !userNotFound() -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it just hide the loading-icon SVG? If yes, can we completely remove it via JavaScript? Because it seems to be related to this report: wireapp/wire-desktop#2507 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now there for exactly this reason. The loading-icon will not be in the DOM when it is not needed.

<div class="loading-wrapper">
<loading-icon></loading-icon>
</div>
Expand Down
13 changes: 9 additions & 4 deletions src/script/components/message/EphemeralTimer.test.ts
Expand Up @@ -32,18 +32,23 @@ class EphemeralTimerPage extends TestPage<EphemeralTimerProps> {
describe('EphemeralTimer', () => {
it('shows the icon', () => {
const message = new Message();
message.ephemeral_started(Date.now());
message.ephemeral_expires(new Date(new Date().getTime() + 10 * 60000).getTime());
const remaining = 600_000;
const now = Date.now();
message.ephemeral_started(now);
message.ephemeral_expires(now + remaining);
message.ephemeral_remaining(remaining);

const ephemeralTimer = new EphemeralTimerPage({message});
const circle = ephemeralTimer.getCircle();
expect(circle.props().style.animationDuration).toBe('600s');

expect(circle.props().style['--offset']).toBe(1);
});
it('hides the icon when no ephemeral timer was started', () => {
const message = new Message();

const ephemeralTimer = new EphemeralTimerPage({message});
const circle = ephemeralTimer.getCircle();
expect(circle.props().style.animationDuration).toBe('0s');

expect(circle.props().style['--offset']).toBe(0);
});
});
15 changes: 10 additions & 5 deletions src/script/components/message/EphemeralTimer.tsx
Expand Up @@ -20,15 +20,20 @@
import React from 'react';

import type {Message} from '../../entity/message/Message';
import {registerReactComponent} from 'Util/ComponentUtil';
import {registerReactComponent, useKoSubscribableChildren} from 'Util/ComponentUtil';

export interface EphemeralTimerProps {
message: Message;
}

const EphemeralTimer: React.FC<EphemeralTimerProps> = ({message}) => {
const started = message.ephemeral_started();
const duration = ((message.ephemeral_expires() as number) - started) / 1000;
const {
ephemeral_remaining: remaining,
ephemeral_started: started,
ephemeral_expires: expires,
} = useKoSubscribableChildren(message, ['ephemeral_remaining', 'ephemeral_started', 'ephemeral_expires']);

const duration = expires - started;

return (
<svg className="ephemeral-timer" viewBox="0 0 8 8" width={8} height={8}>
Expand All @@ -39,7 +44,7 @@ const EphemeralTimer: React.FC<EphemeralTimerProps> = ({message}) => {
cx={4}
cy={4}
r={2}
style={{animationDelay: `${(started - Date.now()) / 1000}s`, animationDuration: `${duration}s`}}
style={{'--offset': remaining / duration || 0} as React.CSSProperties}
transform="rotate(-90 4 4)"
/>
</svg>
Expand All @@ -49,6 +54,6 @@ const EphemeralTimer: React.FC<EphemeralTimerProps> = ({message}) => {
export default EphemeralTimer;

registerReactComponent('ephemeral-timer', {
bindings: 'message',
component: EphemeralTimer,
template: '<div data-bind="react: {message}"></div>',
});
2 changes: 1 addition & 1 deletion src/script/conversation/ConversationEphemeralHandler.ts
Expand Up @@ -48,7 +48,7 @@ export class ConversationEphemeralHandler extends AbstractConversationEventHandl

static get CONFIG() {
return {
INTERVAL_TIME: TIME_IN_MILLIS.SECOND * 0.25,
INTERVAL_TIME: TIME_IN_MILLIS.SECOND,
TIMER_RANGE: {
MAX: TIME_IN_MILLIS.YEAR,
MIN: TIME_IN_MILLIS.SECOND,
Expand Down
2 changes: 0 additions & 2 deletions src/script/entity/message/Message.ts
Expand Up @@ -57,7 +57,6 @@ export class Message {
public reaction: ReactionType;
public readonly accent_color: ko.PureComputed<string>;
public readonly ephemeral_caption: ko.PureComputed<string>;
public readonly ephemeral_duration: ko.Observable<number>;
public readonly ephemeral_expires: ko.Observable<boolean | number | string>;
public readonly ephemeral_remaining: ko.Observable<number>;
public readonly ephemeral_started: ko.Observable<number>;
Expand Down Expand Up @@ -93,7 +92,6 @@ export class Message {
const remainingTime = this.ephemeral_remaining();
return remainingTime ? `${formatDurationCaption(remainingTime)} ${t('ephemeralRemaining')}` : '';
});
this.ephemeral_duration = ko.observable(0);
this.ephemeral_remaining = ko.observable(0);
this.ephemeral_expires = ko.observable(false);
this.ephemeral_started = ko.observable(0);
Expand Down
14 changes: 2 additions & 12 deletions src/style/components/ephemeral-timer.less
Expand Up @@ -28,21 +28,11 @@
}

&__dial {
animation-name: dial-animation;
animation-timing-function: steps(40);
--offset: 1;
fill: none;
stroke: var(--foreground);
stroke-dasharray: @strokelength;
stroke-dashoffset: @strokelength;
stroke-dashoffset: calc(@strokelength * (1 + var(--offset)));
stroke-width: @strokewidth;
}
}

@keyframes dial-animation {
0% {
stroke-dashoffset: @strokelength * 2;
}
100% {
stroke-dashoffset: @strokelength;
}
}