mirror of
https://github.com/block/buzz.git
synced 2026-11-08 18:31:56 +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>
346 lines
12 KiB
Dart
346 lines
12 KiB
Dart
import 'package:buzz/shared/widgets/buzz_agent_icon.dart';
|
|
import 'package:buzz/features/channels/message_content.dart';
|
|
import 'package:buzz/features/channels/media_viewer_page.dart';
|
|
import 'package:buzz/shared/custom_emoji/custom_emoji.dart';
|
|
import 'package:buzz/shared/custom_emoji/custom_emoji_provider.dart';
|
|
import 'package:buzz/shared/custom_emoji/custom_emoji_render.dart';
|
|
import 'package:buzz/shared/emoji/emoji_only.dart';
|
|
import 'package:buzz/shared/theme/theme.dart';
|
|
import 'package:flutter/material.dart';
|
|
import 'package:flutter_test/flutter_test.dart';
|
|
import 'package:gpt_markdown/gpt_markdown.dart';
|
|
import 'package:hooks_riverpod/hooks_riverpod.dart';
|
|
|
|
class _Fixture {
|
|
final revision = ValueNotifier(0);
|
|
String content = 'Hello @Alice, visit #sample.';
|
|
Map<String, String> mentions = {'alice-key': 'Alice'};
|
|
final channels = {'sample': 'channel-one'};
|
|
Set<String> agents = {};
|
|
List<List<String>> tags = [];
|
|
List<CustomEmoji> palette = [];
|
|
bool scaleEmoji = false;
|
|
bool mentionHandler = true;
|
|
bool mediaHandlers = false;
|
|
bool bubble = false;
|
|
bool ownMessage = false;
|
|
int? tappedRevision;
|
|
String? tappedId;
|
|
|
|
Widget build() => ProviderScope(
|
|
overrides: [customEmojiListProvider.overrideWith((ref) => palette)],
|
|
child: MaterialApp(
|
|
theme: AppTheme.light(),
|
|
home: AppMarkdownTheme(
|
|
child: Scaffold(
|
|
body: ValueListenableBuilder<int>(
|
|
valueListenable: revision,
|
|
builder: (_, generation, _) => MessageContent(
|
|
content: content,
|
|
mentionNames: Map.of(mentions),
|
|
channelNames: Map.of(channels),
|
|
agentMentionPubkeys: Set.of(agents),
|
|
tags: tags,
|
|
scaleEmojiOnly: scaleEmoji,
|
|
bubble: bubble,
|
|
ownMessage: ownMessage,
|
|
onMediaReply: mediaHandlers
|
|
? () {
|
|
tappedRevision = generation;
|
|
tappedId = 'reply';
|
|
}
|
|
: null,
|
|
onMediaMore: mediaHandlers
|
|
? (_, url) {
|
|
tappedRevision = generation;
|
|
tappedId = url;
|
|
}
|
|
: null,
|
|
onMentionTap: mentionHandler
|
|
? (id) {
|
|
tappedRevision = generation;
|
|
tappedId = id;
|
|
}
|
|
: null,
|
|
onChannelTap: (id) {
|
|
tappedRevision = generation;
|
|
tappedId = id;
|
|
},
|
|
),
|
|
),
|
|
),
|
|
),
|
|
),
|
|
);
|
|
|
|
Future<void> refresh(WidgetTester tester) async {
|
|
revision.value++;
|
|
await tester.pump();
|
|
}
|
|
}
|
|
|
|
List<MarkdownComponent> _components(WidgetTester tester) => tester
|
|
.widget<GptMarkdown>(find.byType(GptMarkdown).first)
|
|
.inlineComponents!;
|
|
|
|
GptMarkdownThemeData _bubbleTheme(WidgetTester tester) => tester
|
|
.widget<GptMarkdownTheme>(
|
|
find
|
|
.ancestor(
|
|
of: find.byType(GptMarkdown).first,
|
|
matching: find.byType(GptMarkdownTheme),
|
|
)
|
|
.first,
|
|
)
|
|
.gptThemeData;
|
|
|
|
InlineSpan _renderedText(WidgetTester tester) => tester
|
|
.widget<RichText>(
|
|
find
|
|
.descendant(
|
|
of: find.byType(GptMarkdown).first,
|
|
matching: find.byType(RichText),
|
|
)
|
|
.first,
|
|
)
|
|
.text;
|
|
|
|
void main() {
|
|
for (final own in [false, true]) {
|
|
testWidgets(
|
|
'a bubble rebuild with unchanged inputs keeps its parsed markdown '
|
|
'(own: $own)',
|
|
(tester) async {
|
|
final fixture = _Fixture()
|
|
..bubble = true
|
|
..ownMessage = own
|
|
..content = '# Title\n\nHello `code` and #sample';
|
|
addTearDown(fixture.revision.dispose);
|
|
await tester.pumpWidget(fixture.build());
|
|
final theme = _bubbleTheme(tester);
|
|
final text = _renderedText(tester);
|
|
for (var i = 0; i < 3; i++) {
|
|
await fixture.refresh(tester);
|
|
// A new theme object would notify every GptMarkdown below it, and
|
|
// each one would parse its text again.
|
|
expect(_bubbleTheme(tester), same(theme));
|
|
expect(_renderedText(tester), same(text));
|
|
}
|
|
fixture.ownMessage = !own;
|
|
await fixture.refresh(tester);
|
|
expect(_bubbleTheme(tester), isNot(same(theme)));
|
|
expect(_bubbleTheme(tester).linkColor, isNot(theme.linkColor));
|
|
},
|
|
);
|
|
}
|
|
|
|
testWidgets('explicit channel links retain the current callback', (
|
|
tester,
|
|
) async {
|
|
const id = '580ca78b-9dae-46f3-8854-bd671853ba32';
|
|
final fixture = _Fixture()..content = '[Open channel](buzz://channel/$id)';
|
|
addTearDown(fixture.revision.dispose);
|
|
await tester.pumpWidget(fixture.build());
|
|
final before = _components(tester);
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), same(before));
|
|
await tester.tap(find.text('Open channel', findRichText: true));
|
|
expect(fixture.tappedId, id);
|
|
expect(fixture.tappedRevision, fixture.revision.value);
|
|
});
|
|
|
|
testWidgets(
|
|
'inline media retains current actions and invalidates metadata and action presence',
|
|
(tester) async {
|
|
const url = 'https://relay.example/image.png';
|
|
final fixture = _Fixture()
|
|
..content = '\n\nAfter image'
|
|
..mediaHandlers = true
|
|
..tags = [
|
|
['imeta', 'url $url', 'm image/png', 'alt First label'],
|
|
];
|
|
addTearDown(fixture.revision.dispose);
|
|
await tester.pumpWidget(fixture.build());
|
|
final before = _components(tester);
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), same(before));
|
|
await tester.tap(
|
|
find.byKey(const ValueKey('message-media-image-preview:$url')),
|
|
);
|
|
await tester.pumpAndSettle();
|
|
var viewer = tester.widget<MediaImageViewerPage>(
|
|
find.byType(MediaImageViewerPage),
|
|
);
|
|
viewer.onReply!();
|
|
expect(fixture.tappedId, 'reply');
|
|
expect(fixture.tappedRevision, fixture.revision.value);
|
|
viewer.onMore!(tester.element(find.byType(MediaImageViewerPage)), url);
|
|
expect(fixture.tappedId, url);
|
|
expect(fixture.tappedRevision, fixture.revision.value);
|
|
Navigator.of(tester.element(find.byType(MediaImageViewerPage))).pop();
|
|
await tester.pumpAndSettle();
|
|
fixture.tags[0][3] = 'alt Second label';
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), isNot(same(before)));
|
|
await tester.tap(
|
|
find.byKey(const ValueKey('message-media-image-preview:$url')),
|
|
);
|
|
await tester.pumpAndSettle();
|
|
viewer = tester.widget<MediaImageViewerPage>(
|
|
find.byType(MediaImageViewerPage),
|
|
);
|
|
expect(viewer.semanticLabel, 'Second label');
|
|
Navigator.of(tester.element(find.byType(MediaImageViewerPage))).pop();
|
|
await tester.pumpAndSettle();
|
|
final beforeRemoval = _components(tester);
|
|
fixture.mediaHandlers = false;
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), isNot(same(beforeRemoval)));
|
|
await tester.tap(
|
|
find.byKey(const ValueKey('message-media-image-preview:$url')),
|
|
);
|
|
await tester.pumpAndSettle();
|
|
viewer = tester.widget<MediaImageViewerPage>(
|
|
find.byType(MediaImageViewerPage),
|
|
);
|
|
expect(viewer.onReply, isNull);
|
|
expect(viewer.onMore, isNull);
|
|
},
|
|
);
|
|
|
|
testWidgets(
|
|
'equal presentation retains parser components and current callbacks',
|
|
(tester) async {
|
|
final fixture = _Fixture();
|
|
addTearDown(fixture.revision.dispose);
|
|
await tester.pumpWidget(fixture.build());
|
|
final before = _components(tester);
|
|
for (var i = 0; i < 3; i++) {
|
|
await fixture.refresh(tester);
|
|
// This is the production dependency seam: gpt_markdown's config.isSame
|
|
// compares these component identities before deciding whether to parse.
|
|
expect(_components(tester), same(before));
|
|
}
|
|
await tester.tap(find.text('Alice'));
|
|
expect(fixture.tappedRevision, fixture.revision.value);
|
|
expect(fixture.tappedId, 'alice-key');
|
|
await tester.tap(find.text('sample'));
|
|
expect(fixture.tappedRevision, fixture.revision.value);
|
|
expect(fixture.tappedId, 'channel-one');
|
|
|
|
fixture.channels['sample'] = 'channel-two';
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), isNot(same(before)));
|
|
await tester.tap(find.text('sample'));
|
|
expect(fixture.tappedId, 'channel-two');
|
|
},
|
|
);
|
|
|
|
testWidgets(
|
|
'mention labels, bindings, agent status and handler presence invalidate',
|
|
(tester) async {
|
|
final fixture = _Fixture();
|
|
addTearDown(fixture.revision.dispose);
|
|
await tester.pumpWidget(fixture.build());
|
|
var before = _components(tester);
|
|
fixture.mentions['alice-key'] = 'ALICE';
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), isNot(same(before)));
|
|
expect(find.text('ALICE'), findsOneWidget);
|
|
before = _components(tester);
|
|
fixture.agents.add('alice-key');
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), isNot(same(before)));
|
|
expect(find.byType(BuzzAgentIcon), findsOneWidget);
|
|
before = _components(tester);
|
|
fixture.mentionHandler = false;
|
|
fixture.tappedId = null;
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), isNot(same(before)));
|
|
await tester.tap(find.text('ALICE'));
|
|
expect(fixture.tappedId, isNull);
|
|
fixture.mentionHandler = true;
|
|
fixture.mentions = {'second-key': 'ALICE'};
|
|
await fixture.refresh(tester);
|
|
await tester.tap(find.text('ALICE'));
|
|
expect(fixture.tappedId, 'second-key');
|
|
expect(find.byType(BuzzAgentIcon), findsNothing);
|
|
},
|
|
);
|
|
|
|
testWidgets('signed qualified mention changes cannot retain an old binding', (
|
|
tester,
|
|
) async {
|
|
final key = 'a' * 64;
|
|
final fixture = _Fixture()
|
|
..content = '@Alice ($key)'
|
|
..mentions = {}
|
|
..tags = [
|
|
['p', key],
|
|
];
|
|
addTearDown(fixture.revision.dispose);
|
|
await tester.pumpWidget(fixture.build());
|
|
final before = _components(tester);
|
|
await tester.tap(find.text('Alice (aaaaaaaa…aaaa)'));
|
|
expect(fixture.tappedId, key);
|
|
fixture.tags = [];
|
|
fixture.tappedId = null;
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), isNot(same(before)));
|
|
expect(find.text('Alice (aaaaaaaa…aaaa)'), findsNothing);
|
|
});
|
|
|
|
testWidgets('community emoji palette and emoji-only size invalidate', (
|
|
tester,
|
|
) async {
|
|
final fixture = _Fixture()
|
|
..content = ':sample:'
|
|
..palette = [
|
|
const CustomEmoji(
|
|
shortcode: 'sample',
|
|
url: 'https://relay.example/one.png',
|
|
),
|
|
];
|
|
addTearDown(fixture.revision.dispose);
|
|
await tester.pumpWidget(fixture.build());
|
|
var before = _components(tester);
|
|
expect(
|
|
tester.widget<CustomEmojiImage>(find.byType(CustomEmojiImage)).url,
|
|
'https://relay.example/one.png',
|
|
);
|
|
fixture.palette = [
|
|
const CustomEmoji(
|
|
shortcode: 'sample',
|
|
url: 'https://relay.example/two.png',
|
|
),
|
|
];
|
|
ProviderScope.containerOf(
|
|
tester.element(find.byType(MessageContent)),
|
|
).invalidate(customEmojiListProvider);
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), isNot(same(before)));
|
|
expect(
|
|
tester.widget<CustomEmojiImage>(find.byType(CustomEmojiImage)).url,
|
|
'https://relay.example/two.png',
|
|
);
|
|
before = _components(tester);
|
|
fixture.scaleEmoji = true;
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), isNot(same(before)));
|
|
expect(
|
|
tester.widget<CustomEmojiImage>(find.byType(CustomEmojiImage)).size,
|
|
kEmojiOnlyCustomEmojiSize,
|
|
);
|
|
before = _components(tester);
|
|
fixture.tags = [
|
|
['emoji', 'sample', 'https://relay.example/tag.png'],
|
|
];
|
|
await fixture.refresh(tester);
|
|
expect(_components(tester), isNot(same(before)));
|
|
expect(
|
|
tester.widget<CustomEmojiImage>(find.byType(CustomEmojiImage)).url,
|
|
'https://relay.example/tag.png',
|
|
);
|
|
});
|
|
}
|