mirror of
https://github.com/block/buzz.git
synced 2026-11-06 17:31:19 +00:00
## Context Each message in a channel shows its text as Markdown. The `GptMarkdown` widget parses that text. Parsing is slow for long messages. `GptMarkdownTheme` gives Markdown colours and styles to the widgets below it. When it gets a new theme data object, it tells all `GptMarkdown` widgets below it to parse their text again. It compares the old and new objects by identity, because `GptMarkdownThemeData` has no value equality. Thus a new object with the same values also causes a parse. ## Problem Two widgets made a new theme data object on each build: - `MessageContent` made one for each message bubble. - `AppMarkdownTheme` made one for the full app. Thus each rebuild parsed all messages on the screen again. Many events cause a rebuild: a batch of profiles, a read marker, a keyboard frame. In a large channel such as #4things (1,235 members), these events occur almost all the time. ## Change - `MessageContent` keeps its bubble theme until one of its inputs changes. The inputs are: the app Markdown theme, the link colour, own or other message, the ink colour and `onSurface`. - `AppMarkdownTheme` makes new theme data only when `ThemeData` changes or when an outer Markdown theme changes. A change of colour, of brightness, or of own or other message still makes new theme data. Then the Markdown parses again with the new colours. This is correct. ### Related issue None found. ## Testing - New regression tests fail on main. They pass on this branch. - `message_content_stability_test.dart`: When a bubble rebuilds with the same inputs, it keeps the same theme object and the same parsed `TextSpan`. The test does this for own messages and for other messages. When own or other changes, the bubble gets a new theme. - `app_markdown_theme_test.dart`: The app keeps one theme object when it rebuilds. It makes a new one when light or dark mode changes. - `just mobile-install mobile-check mobile-test` passes. - Device test: iPhone 12 mini, Profile build, Tom's Block community, open #4things. The two runs had different live traffic and different cached content. Thus they show the cause and the fix. They do not give a controlled speed measurement. - Before: In the first 2.4 s, parses from rebuilds (`performRebuild`) used approximately 90 ms. One frame used 116 ms. Approximately 48 ms of that frame parsed visible messages again. - After: In 30 s, rebuilds caused no parses. Only the first build of each message (`_firstBuild`) parsed. - An independent agent review found no defects. It found one small item: `ChatColors` has no value equality. Thus a rebuild of `MaterialApp` still makes new app theme data one time. This costs only a little performance. We did not change it. Not tested: A human test did not occur. The mobile testing exception waives it, and an agent did the device test. No test occurred on an Android device. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
121 lines
4.2 KiB
Dart
121 lines
4.2 KiB
Dart
import 'package:flutter/material.dart';
|
|
import 'package:flutter_test/flutter_test.dart';
|
|
import 'package:gpt_markdown/gpt_markdown.dart';
|
|
import 'package:buzz/shared/theme/app_markdown_theme.dart';
|
|
import 'package:buzz/shared/theme/app_theme.dart';
|
|
import 'package:buzz/shared/theme/code_style.dart';
|
|
|
|
/// The Markdown theme a widget reads under [theme], with [wrapped] deciding
|
|
/// whether the app states its code style above it.
|
|
Future<GptMarkdownThemeData> _markdownTheme(
|
|
WidgetTester tester,
|
|
ThemeData theme, {
|
|
required bool wrapped,
|
|
}) async {
|
|
late GptMarkdownThemeData read;
|
|
final reader = Builder(
|
|
builder: (context) {
|
|
read = GptMarkdownTheme.of(context);
|
|
return const SizedBox.shrink();
|
|
},
|
|
);
|
|
|
|
await tester.pumpWidget(
|
|
MaterialApp(
|
|
theme: theme,
|
|
home: wrapped ? AppMarkdownTheme(child: reader) : reader,
|
|
),
|
|
);
|
|
// MaterialApp animates a theme change, so the first frame after a pump can
|
|
// still carry the previous one.
|
|
await tester.pumpAndSettle();
|
|
return read;
|
|
}
|
|
|
|
void main() {
|
|
testWidgets('keeps one theme object until the app theme changes', (
|
|
tester,
|
|
) async {
|
|
final theme = ValueNotifier(AppTheme.light());
|
|
final revision = ValueNotifier(0);
|
|
addTearDown(theme.dispose);
|
|
addTearDown(revision.dispose);
|
|
final reads = <GptMarkdownThemeData>[];
|
|
await tester.pumpWidget(
|
|
ValueListenableBuilder<ThemeData>(
|
|
valueListenable: theme,
|
|
builder: (_, data, _) => MaterialApp(
|
|
theme: data,
|
|
home: ValueListenableBuilder<int>(
|
|
valueListenable: revision,
|
|
// A fresh subtree each time, as an ordinary ancestor rebuild gives.
|
|
builder: (_, generation, _) => AppMarkdownTheme(
|
|
child: Builder(
|
|
key: ValueKey(generation),
|
|
builder: (context) {
|
|
reads.add(GptMarkdownTheme.of(context));
|
|
return const SizedBox.shrink();
|
|
},
|
|
),
|
|
),
|
|
),
|
|
),
|
|
),
|
|
);
|
|
await tester.pumpAndSettle();
|
|
final first = reads.last;
|
|
for (var i = 0; i < 3; i++) {
|
|
revision.value++;
|
|
await tester.pump();
|
|
// GptMarkdownTheme notifies by identity: a new object would make every
|
|
// GptMarkdown in the app parse its text again.
|
|
expect(reads.last, same(first));
|
|
}
|
|
theme.value = AppTheme.dark();
|
|
await tester.pumpAndSettle();
|
|
expect(reads.last, isNot(same(first)));
|
|
expect(reads.last.inlineCode.color, AppTheme.dark().colorScheme.onSurface);
|
|
});
|
|
|
|
testWidgets('states inline code and leaves the rest of Markdown alone', (
|
|
tester,
|
|
) async {
|
|
for (final theme in [AppTheme.light(), AppTheme.dark()]) {
|
|
final ambient = await _markdownTheme(tester, theme, wrapped: false);
|
|
final stated = await _markdownTheme(tester, theme, wrapped: true);
|
|
|
|
// Headings, rules and the rest keep the values gpt_markdown derives from
|
|
// the app theme: a code-only change must not restyle anything else.
|
|
expect(stated.h1, ambient.h1);
|
|
expect(stated.h2, ambient.h2);
|
|
expect(stated.h3, ambient.h3);
|
|
expect(stated.h4, ambient.h4);
|
|
expect(stated.h5, ambient.h5);
|
|
expect(stated.h6, ambient.h6);
|
|
expect(stated.hrLineColor, ambient.hrLineColor);
|
|
expect(stated.hrLineThickness, ambient.hrLineThickness);
|
|
expect(stated.hrLinePadding, ambient.hrLinePadding);
|
|
expect(stated.highlightColor, ambient.highlightColor);
|
|
expect(stated.linkColor, ambient.linkColor);
|
|
expect(stated.linkHoverColor, ambient.linkHoverColor);
|
|
expect(
|
|
stated.autoAddDividerLineAfterH1,
|
|
ambient.autoAddDividerLineAfterH1,
|
|
);
|
|
expect(stated.styleSheet, ambient.styleSheet);
|
|
|
|
// Inline code is the whole of the difference.
|
|
final scheme = theme.colorScheme;
|
|
expect(stated.inlineCode.fontFamily, CodeStyle.fontFamily);
|
|
expect(stated.inlineCode.color, scheme.onSurface);
|
|
expect(stated.inlineCode.backgroundColor, CodeStyle.background(scheme));
|
|
expect(stated.inlineCode.borderColor, CodeStyle.border(scheme));
|
|
expect(
|
|
ambient.inlineCode.fontFamily,
|
|
isNot(CodeStyle.fontFamily),
|
|
reason: 'the package default is what this widget exists to replace',
|
|
);
|
|
}
|
|
});
|
|
}
|