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

Cherry Pick: [SuperEditor] Add test to demonstrate that changing layer builders does not re-layout #2466

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 40 additions & 28 deletions super_editor/lib/src/infrastructure/content_layers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -271,34 +271,32 @@ class ContentLayersElement extends RenderObjectElement {
super.markNeedsBuild();
}

/// Builds the underlays and overlays.
void buildLayers() {
contentLayersLog.finer("ContentLayersElement - (re)building layers");

owner!.buildScope(this, () {
final List<Element> underlays = List<Element>.filled(widget.underlays.length, _NullElement.instance);
for (int i = 0; i < underlays.length; i += 1) {
late final Element child;
if (i > _underlays.length - 1) {
child = inflateWidget(widget.underlays[i](this), _UnderlaySlot(i));
} else {
child = super.updateChild(_underlays[i], widget.underlays[i](this), _UnderlaySlot(i))!;
}
underlays[i] = child;
final List<Element> underlays = List<Element>.filled(widget.underlays.length, _NullElement.instance);
for (int i = 0; i < underlays.length; i += 1) {
late final Element child;
if (i > _underlays.length - 1) {
child = inflateWidget(widget.underlays[i](this), _UnderlaySlot(i));
} else {
child = super.updateChild(_underlays[i], widget.underlays[i](this), _UnderlaySlot(i))!;
}
_underlays = underlays;

final List<Element> overlays = List<Element>.filled(widget.overlays.length, _NullElement.instance);
for (int i = 0; i < overlays.length; i += 1) {
late final Element child;
if (i > _overlays.length - 1) {
child = inflateWidget(widget.overlays[i](this), _OverlaySlot(i));
} else {
child = super.updateChild(_overlays[i], widget.overlays[i](this), _OverlaySlot(i))!;
}
overlays[i] = child;
underlays[i] = child;
}
_underlays = underlays;

final List<Element> overlays = List<Element>.filled(widget.overlays.length, _NullElement.instance);
for (int i = 0; i < overlays.length; i += 1) {
late final Element child;
if (i > _overlays.length - 1) {
child = inflateWidget(widget.overlays[i](this), _OverlaySlot(i));
} else {
child = super.updateChild(_overlays[i], widget.overlays[i](this), _OverlaySlot(i))!;
}
_overlays = overlays;
});
overlays[i] = child;
}
_overlays = overlays;
}

@override
Expand Down Expand Up @@ -345,9 +343,19 @@ class ContentLayersElement extends RenderObjectElement {
assert(!debugChildrenHaveDuplicateKeys(widget, [newContent]));

_content = updateChild(_content, newContent, _contentSlot);
// super.update() and updateChild() is where the framework reparents
// forgotten children. Therefore, at this point, the framework is
// done with the concept of forgotten children, so we clear our

if (!renderObject.contentNeedsLayout) {
// Layout has already run. No layout bounds changed. There might be a
// non-layout change that needs to be painted, e.g., change to theme brightness.
// Re-build all layers, which is safe to do because no layout constraints changed.
buildLayers();
}
// Else, dirty content layout will cause this whole widget to re-layout. The
// layers will be re-built during that layout pass.

// super.update() and updateChild() is where the framework reparents
// forgotten children. Therefore, at this point, the framework is
// done with the concept of forgotten children, so we clear our
// local cache of them, too.
_forgottenChildren.clear();
}
Expand Down Expand Up @@ -644,7 +652,11 @@ class RenderContentLayers extends RenderSliver with RenderSliverHelpers {
// content changes.
contentLayersLog.fine("Building layers");
invokeLayoutCallback((constraints) {
_element!.buildLayers();
// Usually, widgets are built during the build phase, but we're building the layers
// during layout phase, so we need to explicitly tell Flutter to build all elements.
_element!.owner!.buildScope(_element!, () {
_element!.buildLayers();
});
});
contentLayersLog.finer("Done building layers");

Expand Down
103 changes: 103 additions & 0 deletions super_editor/test/super_editor/supereditor_theme_switching_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter_test_runners/flutter_test_runners.dart';
import 'package:super_editor/super_editor.dart';
import 'package:super_editor/super_editor_test.dart';

import 'test_documents.dart';

void main() {
group('SuperEditor > theme switching', () {
testWidgetsOnArbitraryDesktop('switches caret color', (tester) async {
final brightnessNotifier = ValueNotifier<Brightness>(Brightness.light);

await _pumpThemeSwitchingTestApp(tester, brightnessNotifier: brightnessNotifier);

// Place the caret at the beginning of the paragraph.
await tester.placeCaretInParagraph('1', 0);

// Ensure the caret is green, because the theme is light.
expect(_findDesktopCaretColor(tester), Colors.green.shade500);

// Switch the theme to dark.
brightnessNotifier.value = Brightness.dark;
await tester.pumpAndSettle();

// Ensure the caret is red, because the theme is dark.
expect(_findDesktopCaretColor(tester), Colors.red.shade500);
});

testWidgetsOnArbitraryDesktop('switches caret color after typing', (tester) async {
final brightnessNotifier = ValueNotifier<Brightness>(Brightness.light);

await _pumpThemeSwitchingTestApp(tester, brightnessNotifier: brightnessNotifier);

// Place the caret at the beginning of the paragraph.
await tester.placeCaretInParagraph('1', 0);

// Ensure the caret is green, because the theme is light.
expect(_findDesktopCaretColor(tester), Colors.green.shade500);

// Switch the theme to dark.
brightnessNotifier.value = Brightness.dark;
await tester.pumpAndSettle();

// Type a character to trigger a re-layout.
await tester.typeImeText('a');

// Ensure the caret is red, because the theme is dark.
expect(_findDesktopCaretColor(tester), Colors.red.shade500);
});
});
}

/// Pumps a widget tree that rebuilds when the [brightnessNotifier] changes.
///
/// The widget tree contains a [SuperEditor] with a custom caret overlay that
/// changes color based on the brightness of the theme.
Future<void> _pumpThemeSwitchingTestApp(
WidgetTester tester, {
required ValueNotifier<Brightness> brightnessNotifier,
}) async {
final composer = MutableDocumentComposer();
final editor = createDefaultDocumentEditor(
document: singleParagraphDoc(),
composer: composer,
);

await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: ValueListenableBuilder(
valueListenable: brightnessNotifier,
builder: (context, brightness, child) {
return Theme(
data: ThemeData(
brightness: brightness,
),
child: SuperEditor(
editor: editor,
documentOverlayBuilders: [
// Copy all default overlay builders except the caret overlay builder.
...defaultSuperEditorDocumentOverlayBuilders.where(
(builder) => builder is! DefaultCaretOverlayBuilder,
),
DefaultCaretOverlayBuilder(
caretStyle: CaretStyle(
color: brightness == Brightness.light ? Colors.green : Colors.red,
),
)
],
),
);
},
),
),
),
);
}

Color _findDesktopCaretColor(WidgetTester tester) {
final caret = tester.widget<Container>(find.byKey(DocumentKeys.caret));
return (caret.decoration as BoxDecoration).color!;
}
Loading