mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 16:06:32 +00:00
fix: stop each keystroke in the composer re-rendering the whole screen twice
The editor page posted its full `innerHTML` on every input event and its formatting state on every selection change, which fires on every keystroke too. Each message re-rendered the whole composer, where a `JSON.stringify` snapshot of the body ran on every render (PF8). - typing now posts at most one `change` per 400 ms: the first keystroke at once, so the composer knows it is dirty, then the latest content when the window ends; toolbar commands and blur post straight away - the formatting state is posted only when it changes - the empty-placeholder check no longer serializes the body per keystroke, and the dirty-tracking snapshot is memoized - autosave reads the live editor DOM, like send, save-and-close and the format switch already do, so the throttle can't make it save a slightly older body The body stays in state rather than a ref: the Send button and the close guard render from it.
This commit is contained in:
@@ -1,5 +1,7 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { buildEditorCsp, buildEditorHtml, shouldBlockEditorRemoteImages } from '../editor-html';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import {
|
||||
buildEditorCsp, buildEditorHtml, CHANGE_THROTTLE_MS, shouldBlockEditorRemoteImages,
|
||||
} from '../editor-html';
|
||||
import { LIGHT_COLORS, DARK_COLORS } from '../../theme/tokens';
|
||||
|
||||
function extractScript(html: string): string {
|
||||
@@ -120,3 +122,77 @@ describe('shouldBlockEditorRemoteImages', () => {
|
||||
expect(shouldBlockEditorRemoteImages({ ...opts, seedHtml: '<p>Hi</p><img src="https://me.example/logo.png">' })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// Runs the page script against a minimal fake DOM to check what it posts to
|
||||
// RN while the user types (PF8: each post re-renders the whole composer).
|
||||
describe('editor page messages', () => {
|
||||
function boot() {
|
||||
const listeners: Record<string, Array<() => void>> = {};
|
||||
const docListeners: Record<string, Array<() => void>> = {};
|
||||
const posted: Array<{ type: string; payload: unknown }> = [];
|
||||
const editor = {
|
||||
innerHTML: '',
|
||||
get textContent() { return this.innerHTML.replace(/<[^>]*>/g, ''); },
|
||||
scrollHeight: 100,
|
||||
querySelector: () => null,
|
||||
setAttribute: () => undefined,
|
||||
focus: () => undefined,
|
||||
addEventListener: (name: string, fn: () => void) => { (listeners[name] ??= []).push(fn); },
|
||||
};
|
||||
const commandState: Record<string, boolean> = {};
|
||||
const document = {
|
||||
activeElement: editor as unknown,
|
||||
getElementById: () => editor,
|
||||
addEventListener: (name: string, fn: () => void) => { (docListeners[name] ??= []).push(fn); },
|
||||
queryCommandState: (name: string) => !!commandState[name],
|
||||
};
|
||||
const window = {
|
||||
ReactNativeWebView: { postMessage: (msg: string) => posted.push(JSON.parse(msg)) },
|
||||
addEventListener: () => undefined,
|
||||
getSelection: () => null,
|
||||
};
|
||||
const script = extractScript(buildEditorHtml({ initialHtml: '<p><br></p>', placeholder: '', c: LIGHT_COLORS }));
|
||||
new Function('window', 'document', script)(window, document);
|
||||
const changes = () => posted.filter((m) => m.type === 'change').map((m) => m.payload);
|
||||
const selections = () => posted.filter((m) => m.type === 'selection');
|
||||
const type = (html: string) => { editor.innerHTML = html; listeners.input.forEach((fn) => fn()); };
|
||||
const fire = (name: string) => (listeners[name] ?? docListeners[name]).forEach((fn) => fn());
|
||||
return { changes, selections, type, fire, commandState, posted };
|
||||
}
|
||||
|
||||
beforeEach(() => { vi.useFakeTimers(); });
|
||||
afterEach(() => { vi.useRealTimers(); });
|
||||
|
||||
it('posts the first keystroke at once and then at most one change per window', () => {
|
||||
const page = boot();
|
||||
const before = page.changes().length;
|
||||
page.type('<p>H</p>');
|
||||
expect(page.changes().slice(before)).toEqual(['<p>H</p>']);
|
||||
page.type('<p>He</p>');
|
||||
page.type('<p>Hel</p>');
|
||||
expect(page.changes().slice(before)).toEqual(['<p>H</p>']);
|
||||
vi.advanceTimersByTime(CHANGE_THROTTLE_MS);
|
||||
expect(page.changes().slice(before)).toEqual(['<p>H</p>', '<p>Hel</p>']);
|
||||
});
|
||||
|
||||
it('flushes a pending change on blur', () => {
|
||||
const page = boot();
|
||||
page.type('<p>A</p>');
|
||||
page.type('<p>AB</p>');
|
||||
page.fire('blur');
|
||||
expect(page.changes().slice(-1)).toEqual(['<p>AB</p>']);
|
||||
vi.advanceTimersByTime(CHANGE_THROTTLE_MS * 2);
|
||||
expect(page.changes().filter((c) => c === '<p>AB</p>')).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('posts the selection state only when the formatting changes', () => {
|
||||
const page = boot();
|
||||
page.fire('selectionchange');
|
||||
page.fire('selectionchange');
|
||||
expect(page.selections()).toHaveLength(1);
|
||||
page.commandState.bold = true;
|
||||
page.fire('selectionchange');
|
||||
expect(page.selections()).toHaveLength(2);
|
||||
expect((page.selections()[1].payload as { bold: boolean }).bold).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
+48
-9
@@ -15,6 +15,11 @@ import type { ThemePalette } from '../theme/tokens';
|
||||
// user types, and the parent ScrollView handles overflow.
|
||||
export const MIN_EDITOR_HEIGHT = 220;
|
||||
|
||||
// Shortest gap between two `change` posts while typing (see the page script).
|
||||
// Readers that need the current body - send, save, format switch - ask the
|
||||
// page with `getHtml` instead of waiting for the next post.
|
||||
export const CHANGE_THROTTLE_MS = 400;
|
||||
|
||||
/**
|
||||
* The editor page's Content-Security-Policy, mirroring the viewer's
|
||||
* (`src/lib/email-html.ts`). Nothing but images, styles and our own inline
|
||||
@@ -134,15 +139,18 @@ export function buildEditorHtml(opts: {
|
||||
var editor = document.getElementById('editor');
|
||||
var lastHtml = null;
|
||||
var lastHeight = 0;
|
||||
var lastSelection = '';
|
||||
// Every 'change' re-renders the whole composer, so typing posts at most
|
||||
// one per window: the first keystroke at once (the composer learns it is
|
||||
// dirty), then the latest content when the window ends. Commands, blur
|
||||
// and setHtml post straight away.
|
||||
var CHANGE_THROTTLE_MS = ${CHANGE_THROTTLE_MS};
|
||||
var lastChangeAt = 0;
|
||||
var changeTimer = null;
|
||||
|
||||
// Checked on every keystroke, so it avoids serializing innerHTML.
|
||||
function refreshEmpty() {
|
||||
var html = editor.innerHTML;
|
||||
var isEmpty =
|
||||
html === '' ||
|
||||
html === '<br>' ||
|
||||
html === '<p></p>' ||
|
||||
html === '<p><br></p>' ||
|
||||
editor.textContent.trim() === '' && editor.querySelectorAll('img').length === 0;
|
||||
var isEmpty = editor.textContent.trim() === '' && !editor.querySelector('img');
|
||||
editor.setAttribute('data-empty', isEmpty ? 'true' : 'false');
|
||||
}
|
||||
|
||||
@@ -160,13 +168,36 @@ export function buildEditorHtml(opts: {
|
||||
}
|
||||
|
||||
function reportChange() {
|
||||
if (changeTimer) {
|
||||
clearTimeout(changeTimer);
|
||||
changeTimer = null;
|
||||
}
|
||||
refreshEmpty();
|
||||
var html = editor.innerHTML;
|
||||
if (html === lastHtml) return;
|
||||
lastHtml = html;
|
||||
lastChangeAt = Date.now();
|
||||
post('change', html);
|
||||
}
|
||||
|
||||
function scheduleChange() {
|
||||
refreshEmpty();
|
||||
if (changeTimer) return;
|
||||
var wait = CHANGE_THROTTLE_MS - (Date.now() - lastChangeAt);
|
||||
if (wait <= 0) {
|
||||
reportChange();
|
||||
return;
|
||||
}
|
||||
changeTimer = setTimeout(function () {
|
||||
changeTimer = null;
|
||||
reportChange();
|
||||
}, wait);
|
||||
}
|
||||
|
||||
function flushChange() {
|
||||
if (changeTimer) reportChange();
|
||||
}
|
||||
|
||||
function reportSelection() {
|
||||
function active(name) {
|
||||
try { return document.queryCommandState(name); } catch (e) { return false; }
|
||||
@@ -196,6 +227,11 @@ export function buildEditorHtml(opts: {
|
||||
alignRight: active('justifyRight'),
|
||||
link: inBlock('A'),
|
||||
};
|
||||
// The caret moves on every keystroke; only a change of the formatting
|
||||
// state is worth a message (and a composer re-render).
|
||||
var key = JSON.stringify(state);
|
||||
if (key === lastSelection) return;
|
||||
lastSelection = key;
|
||||
post('selection', state);
|
||||
}
|
||||
|
||||
@@ -206,10 +242,13 @@ export function buildEditorHtml(opts: {
|
||||
lastHtml = editor.innerHTML;
|
||||
|
||||
editor.addEventListener('input', function () {
|
||||
reportChange();
|
||||
scheduleChange();
|
||||
reportHeight();
|
||||
});
|
||||
editor.addEventListener('blur', function () { post('blur', null); });
|
||||
editor.addEventListener('blur', function () {
|
||||
flushChange();
|
||||
post('blur', null);
|
||||
});
|
||||
editor.addEventListener('focus', function () { post('focus', null); });
|
||||
document.addEventListener('selectionchange', function () {
|
||||
if (document.activeElement === editor) reportSelection();
|
||||
|
||||
@@ -1043,10 +1043,13 @@ export default function ComposeScreen({ route, navigation }: Props) {
|
||||
att: s.attachments.filter((a) => !a.inline).map((a) => a.blobId ?? a.localId),
|
||||
});
|
||||
|
||||
const currentSnapshot = snapshotOf({
|
||||
// Memoized: the snapshot copies the whole body, and the screen re-renders
|
||||
// for plenty that doesn't touch it (PF8).
|
||||
const currentSnapshot = React.useMemo(() => snapshotOf({
|
||||
to: finalTo, cc: finalCc, bcc: finalBcc, subject,
|
||||
body: plainTextMode ? plainBody : bodyHtml, attachments,
|
||||
});
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}), [finalTo, finalCc, finalBcc, subject, plainTextMode, plainBody, bodyHtml, attachments]);
|
||||
const baselineRef = React.useRef(snapshotOf({
|
||||
to: initialTo, cc: initialCc, bcc: initialBcc, subject: initialSubject,
|
||||
body: plainTextMode ? initialPlainBody : initialBodyHtml, attachments: initialAttachments,
|
||||
@@ -1322,7 +1325,8 @@ export default function ComposeScreen({ route, navigation }: Props) {
|
||||
saveTimerRef.current = setTimeout(() => {
|
||||
saveTimerRef.current = null;
|
||||
if (sendingRef.current || !latestRef.current.needsSave) return;
|
||||
saveDraftRef.current({ live: false }).catch((err) => {
|
||||
// Live: the editor posts its content throttled, so read the DOM itself.
|
||||
saveDraftRef.current({ live: true }).catch((err) => {
|
||||
console.warn('[compose] autosave failed', err);
|
||||
});
|
||||
}, wait);
|
||||
|
||||
Reference in New Issue
Block a user