mirror of
https://github.com/block/buzz.git
synced 2026-10-27 12:40:30 +00:00
fix(mobile): keep token labels visible in authored Markdown links (#7545)
## Summary On iOS, an authored Markdown link renders as blank when its label is recognized as a Buzz channel, mention, or custom emoji. For example: ```markdown [#2959](https://example.com/) ``` The surrounding text remains visible, but the link has no visible label or tap target. Buzz's `_ChannelLinkMd`, `_MentionMd`, and `CustomEmojiMd` components return `WidgetSpan`s. `gpt_markdown` also renders an authored link through a `WidgetSpan` and reparses its label through the registered components. Because the Buzz components were allowed to run inside link labels, the result was a placeholder nested inside another placeholder, which does not paint on iOS. This PR adds the same scope restriction used by `gpt_markdown`'s built-in link component: ```dart @override Set<MarkdownScope> get scopes => MarkdownComponent.allScopesExceptLinkLabel; ``` The override is applied to all three widget-based components. Authored labels such as `[#2959](url)`, `[@Alice](url)`, and `[👋](url)` therefore remain ordinary link text, while the same tokens outside links continue rendering as Buzz pills and custom emoji. The PR also updates the shared test helpers to find `RichText` subclasses. Since the upgrade to `gpt_markdown` 1.2.1, body paragraphs can render through `BidiRichText`; `find.byType(RichText)` does not match that subclass and could allow an absence assertion to pass without inspecting the message text. The handling of unknown bare `#tokens` is deliberately unchanged. Whether those should remain non-interactive pills or become plain text is a separate product decision. ### Related issue Fixes #6124. This supersedes #6135. That PR solves the same issue by maintaining a custom fork of `gpt_markdown`'s link component; this PR instead uses the scoped-component API already available in `gpt_markdown` 1.2.1. ### Testing Focused suites: ```bash flutter test \ test/features/channels/message_content_test.dart \ test/features/channels/message_content_custom_emoji_test.dart ``` Result: 95 tests passed, 0 failed. Additional checks: - `just mobile-check` — passed; analyzer reported no issues. - `just mobile-fmt` — passed; 549 files formatted with 0 changes. - Six new regression tests cover channel, mention, emoji, plain-link control, and outside-link behavior. - Removing the `_ChannelLinkMd` override causes two regression tests to fail; removing `_MentionMd` or `CustomEmojiMd` causes one test to fail in each case. - Three tests in `message_content_test.dart` directly assert that the nested-placeholder structure is absent. The custom-emoji tests separately verify literal text and the absence or presence of `CustomEmojiImage`, depending on whether the shortcode is inside or outside a link. The complete local mobile suite reports 15 failures that reproduce identically on a clean checkout of `main` at `44316ff7`. In [CI for that same commit](https://github.com/block/buzz/actions/runs/34238736818), `Clients / Mobile`, `Mobile`, and `Mobile Swift Domain / Mobile Swift` all passed. No native-device screenshot is included. The nested-placeholder fix was verified through the rendered widget structure, but an iOS simulator build was blocked by the existing `MLImage.framework` arm64-simulator incompatibility. Signed-off-by: Mohammad Asfour <masfour700@gmail.com> Signed-off-by: Samin Yeasar Sohag <sssohag172@gmail.com> Co-authored-by: Samin Yeasar Sohag <sssohag172@gmail.com>
This commit is contained in:
@@ -1046,6 +1046,13 @@ class _MentionMd extends InlineMd {
|
||||
this.onMentionTap,
|
||||
});
|
||||
|
||||
/// Excluded from link labels: this component renders a [WidgetSpan], and a
|
||||
/// placeholder nested inside the link's own placeholder does not paint on
|
||||
/// iOS — an authored `[@mention](url)` renders as nothing. Link resolution
|
||||
/// wins over token detection inside a label.
|
||||
@override
|
||||
Set<MarkdownScope> get scopes => MarkdownComponent.allScopesExceptLinkLabel;
|
||||
|
||||
@override
|
||||
RegExp get exp => _exp;
|
||||
|
||||
|
||||
@@ -18,6 +18,13 @@ class _ChannelLinkMd extends InlineMd {
|
||||
|
||||
_ChannelLinkMd({required this.channelNames, this.onChannelTap});
|
||||
|
||||
/// Excluded from link labels: this component renders a [WidgetSpan], and a
|
||||
/// placeholder nested inside the link's own placeholder does not paint on
|
||||
/// iOS — an authored `[#channel](url)` renders as nothing. Link resolution
|
||||
/// wins over token detection inside a label.
|
||||
@override
|
||||
Set<MarkdownScope> get scopes => MarkdownComponent.allScopesExceptLinkLabel;
|
||||
|
||||
@override
|
||||
RegExp get exp => _exp;
|
||||
|
||||
|
||||
@@ -65,6 +65,13 @@ class CustomEmojiMd extends InlineMd {
|
||||
this.size = kCustomEmojiInlineSize,
|
||||
}) : _urlByShortcode = _referencedUrls(palette, content);
|
||||
|
||||
/// Excluded from link labels: this component renders a [WidgetSpan], and a
|
||||
/// placeholder nested inside the link's own placeholder does not paint on
|
||||
/// iOS — an authored `[:emoji:](url)` renders as nothing. Link resolution
|
||||
/// wins over token detection inside a label.
|
||||
@override
|
||||
Set<MarkdownScope> get scopes => MarkdownComponent.allScopesExceptLinkLabel;
|
||||
|
||||
// Look ahead so adjacent tokens sharing a colon are both considered:
|
||||
// :unknown:known: must still allow the known token to match.
|
||||
static final _shortcodeScan = RegExp(
|
||||
|
||||
@@ -37,6 +37,34 @@ Widget _testable(String content, {List<List<String>> tags = const []}) {
|
||||
}
|
||||
|
||||
void main() {
|
||||
// Regression: https://github.com/block/buzz/issues/6124
|
||||
//
|
||||
// A custom-emoji WidgetSpan inside a link's own WidgetSpan does not paint on
|
||||
// iOS, so the whole link renders as nothing. `CustomEmojiMd` opts out of
|
||||
// `MarkdownScope.linkLabel`; this fails if that override is removed.
|
||||
testWidgets('an emoji shortcode in a link label stays link text', (
|
||||
tester,
|
||||
) async {
|
||||
await tester.pumpWidget(_testable('Say [:wave:](https://example.com/x)'));
|
||||
|
||||
expect(find.byType(CustomEmojiImage), findsNothing);
|
||||
final text = tester
|
||||
.widgetList<RichText>(
|
||||
find.byWidgetPredicate((widget) => widget is RichText),
|
||||
)
|
||||
.map((widget) => widget.text.toPlainText())
|
||||
.join();
|
||||
expect(text, contains(':wave:'));
|
||||
});
|
||||
|
||||
testWidgets('an emoji shortcode outside a link label still renders', (
|
||||
tester,
|
||||
) async {
|
||||
await tester.pumpWidget(_testable('Say :wave: now'));
|
||||
|
||||
expect(find.byType(CustomEmojiImage), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('message wiring excludes unrelated emoji from the regex', (
|
||||
tester,
|
||||
) async {
|
||||
|
||||
@@ -295,9 +295,49 @@ bool _isImageViewerHeroEnabled(WidgetTester tester) {
|
||||
return tester.widget<HeroMode>(_imageViewerHeroMode()).enabled;
|
||||
}
|
||||
|
||||
/// Whether any placeholder span in the tree contains a further placeholder.
|
||||
///
|
||||
/// This is the shape that renders as nothing on iOS: a token component's
|
||||
/// `WidgetSpan` nested inside the `WidgetSpan` a link is already drawn in.
|
||||
/// Each placeholder becomes its own paragraph, so a paragraph that both sits
|
||||
/// under another paragraph's placeholder and holds a placeholder of its own is
|
||||
/// the defect.
|
||||
bool _hasNestedPlaceholder(WidgetTester tester) {
|
||||
for (final rich in tester.widgetList<RichText>(_anyRichText())) {
|
||||
var placeholders = 0;
|
||||
rich.text.visitChildren((span) {
|
||||
if (span is PlaceholderSpan) placeholders++;
|
||||
return true;
|
||||
});
|
||||
if (placeholders == 0) continue;
|
||||
// A paragraph holding a placeholder is fine on its own — the message body
|
||||
// does that for every link. It is only a defect when that paragraph is
|
||||
// itself inside another paragraph's placeholder.
|
||||
final ancestors = find.ancestor(
|
||||
of: find.byWidget(rich),
|
||||
matching: _anyRichText(),
|
||||
);
|
||||
if (ancestors.evaluate().isNotEmpty) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/// Matches every `RichText` in the tree, including subclasses.
|
||||
///
|
||||
/// gpt_markdown renders body paragraphs through `BidiRichText`, a `RichText`
|
||||
/// subclass. `find.byType` matches the exact runtime type only, so it silently
|
||||
/// skips those paragraphs — an assertion that text is rendered then fails, and
|
||||
/// an assertion that text is absent then passes for the wrong reason.
|
||||
Finder _anyRichText() {
|
||||
return find.byWidgetPredicate(
|
||||
(widget) => widget is RichText,
|
||||
description: 'any RichText (including subclasses)',
|
||||
);
|
||||
}
|
||||
|
||||
/// Extracts all plain text from all RichText widgets in the tree.
|
||||
String _allRichText(WidgetTester tester) {
|
||||
final richTexts = tester.widgetList<RichText>(find.byType(RichText));
|
||||
final richTexts = tester.widgetList<RichText>(_anyRichText());
|
||||
return richTexts.map((rt) => rt.text.toPlainText()).join('\n');
|
||||
}
|
||||
|
||||
@@ -312,7 +352,7 @@ Finder _findRich(String text) {
|
||||
/// Checks that the given text appears as bold (fontWeight >= w600) in some
|
||||
/// TextSpan within any RichText widget.
|
||||
bool _hasBoldSpan(WidgetTester tester, String text) {
|
||||
for (final rt in tester.widgetList<RichText>(find.byType(RichText))) {
|
||||
for (final rt in tester.widgetList<RichText>(_anyRichText())) {
|
||||
if (_spanHasStyle(
|
||||
rt.text,
|
||||
text,
|
||||
@@ -326,7 +366,7 @@ bool _hasBoldSpan(WidgetTester tester, String text) {
|
||||
}
|
||||
|
||||
bool _hasItalicSpan(WidgetTester tester, String text) {
|
||||
for (final rt in tester.widgetList<RichText>(find.byType(RichText))) {
|
||||
for (final rt in tester.widgetList<RichText>(_anyRichText())) {
|
||||
if (_spanHasStyle(rt.text, text, (s) => s.fontStyle == FontStyle.italic)) {
|
||||
return true;
|
||||
}
|
||||
@@ -335,7 +375,7 @@ bool _hasItalicSpan(WidgetTester tester, String text) {
|
||||
}
|
||||
|
||||
bool _hasStrikethroughSpan(WidgetTester tester, String text) {
|
||||
for (final rt in tester.widgetList<RichText>(find.byType(RichText))) {
|
||||
for (final rt in tester.widgetList<RichText>(_anyRichText())) {
|
||||
if (_spanHasStyle(
|
||||
rt.text,
|
||||
text,
|
||||
@@ -3045,6 +3085,89 @@ Photos
|
||||
});
|
||||
});
|
||||
|
||||
// Regression: https://github.com/block/buzz/issues/6124
|
||||
//
|
||||
// gpt_markdown renders a link's label by recursing with
|
||||
// `MarkdownScope.linkLabel`. A token component that claims the label
|
||||
// returns a WidgetSpan, which then sits inside the link's own WidgetSpan
|
||||
// — and a placeholder nested in a placeholder does not paint on iOS, so
|
||||
// the whole link disappears. The components opt out of that scope; these
|
||||
// tests fail if any of the `scopes` overrides is removed.
|
||||
group('authored link labels', () {
|
||||
testWidgets('a #channel label stays link text, not a channel pill', (
|
||||
tester,
|
||||
) async {
|
||||
await tester.pumpWidget(
|
||||
_testable(
|
||||
const MessageContent(
|
||||
content: 'See [#2959](https://example.com/x) for details.',
|
||||
// Known name and generic token both reach the component; use the
|
||||
// known one so the pill would definitely render if not excluded.
|
||||
channelNames: {'2959': 'ch-id-1'},
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
expect(_findRich('#2959'), findsOneWidget);
|
||||
expect(find.byIcon(BuzzIcons.hash), findsNothing);
|
||||
expect(_hasNestedPlaceholder(tester), isFalse);
|
||||
});
|
||||
|
||||
testWidgets('an @mention label stays link text, not a mention pill', (
|
||||
tester,
|
||||
) async {
|
||||
await tester.pumpWidget(
|
||||
_testable(
|
||||
const MessageContent(
|
||||
content: 'Ask [@Alice](https://example.com/x) about it.',
|
||||
mentionNames: {'pk1': 'Alice'},
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
expect(_findRich('@Alice'), findsOneWidget);
|
||||
// The pill splits the label into a separate '@' and name; the link
|
||||
// must not.
|
||||
expect(find.text('@'), findsNothing);
|
||||
expect(_hasNestedPlaceholder(tester), isFalse);
|
||||
});
|
||||
|
||||
testWidgets('a label with no token renders unchanged', (tester) async {
|
||||
await tester.pumpWidget(
|
||||
_testable(
|
||||
const MessageContent(
|
||||
content: 'See [ticket 2959](https://example.com/x) for details.',
|
||||
channelNames: {'2959': 'ch-id-1'},
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
expect(_findRich('ticket 2959'), findsOneWidget);
|
||||
expect(_hasNestedPlaceholder(tester), isFalse);
|
||||
});
|
||||
|
||||
testWidgets('tokens outside a link label still render as pills', (
|
||||
tester,
|
||||
) async {
|
||||
await tester.pumpWidget(
|
||||
_testable(
|
||||
const MessageContent(
|
||||
content: 'See #2959 and @Alice, plus [#2959](https://x.test/y).',
|
||||
channelNames: {'2959': 'ch-id-1'},
|
||||
mentionNames: {'pk1': 'Alice'},
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
// Excluding the link-label scope must not disable the components
|
||||
// everywhere else: the bare tokens keep their pills.
|
||||
expect(find.byIcon(BuzzIcons.hash), findsOneWidget);
|
||||
expect(find.text('@'), findsOneWidget);
|
||||
expect(find.text('Alice'), findsOneWidget);
|
||||
expect(_findRich('#2959'), findsOneWidget);
|
||||
});
|
||||
});
|
||||
|
||||
group('mixed content', () {
|
||||
testWidgets('renders bold with mentions', (tester) async {
|
||||
await tester.pumpWidget(
|
||||
|
||||
Reference in New Issue
Block a user