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

fix: sliver child is text without renderer should not accpet #898

Merged
merged 5 commits into from
Nov 25, 2021
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
22 changes: 22 additions & 0 deletions integration_tests/specs/css/css-display/sliver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,26 @@ describe('display sliver', () => {
await snapshot();
});

it('sliver child is text or comment', async () => {
wssgcg1213 marked this conversation as resolved.
Show resolved Hide resolved
var comment = document.createComment('HelloWorld');
var text = document.createTextNode('HelloWorld');
// Empty text node has different logic in backend.
var emptyText = document.createTextNode('');

var container = createSliverBasicCase();

container.insertBefore(emptyText, container.firstChild);
container.insertBefore(text, container.firstChild);
container.insertBefore(comment, container.firstChild);
expect(container.childNodes.length).toEqual(103);

await snapshot(); // Not throws error is ok.
});

it('sliver child is display none', async () => {
var container = createSliverBasicCase();

container.children[2].style.display = 'none';
await snapshot(); // Not throws error is ok.
});
});
1 change: 1 addition & 0 deletions kraken/lib/dom.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export 'src/dom/text_node.dart';
export 'src/dom/window.dart';
export 'src/dom/document.dart';
export 'src/dom/document_fragment.dart';
export 'src/dom/sliver_manager.dart';

// Elements
export 'src/dom/elements/semantics_text.dart';
Expand Down
13 changes: 7 additions & 6 deletions kraken/lib/src/dom/element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import 'package:kraken/bridge.dart';
import 'package:kraken/css.dart';
import 'package:kraken/dom.dart';
import 'package:kraken/rendering.dart';
import 'package:kraken/src/dom/sliver_manager.dart';
import 'package:meta/meta.dart';

import 'element_native_methods.dart';
Expand Down Expand Up @@ -391,7 +390,9 @@ class Element extends Node
@override
void willAttachRenderer() {
// Init render box model.
createRenderer();
if (renderStyle.display != CSSDisplay.none) {
createRenderer();
}
}

@override
Expand Down Expand Up @@ -571,9 +572,9 @@ class Element extends Node
void attachTo(Node parent, {RenderBox? after}) {
_applyStyle(style);

if (renderStyle.display != CSSDisplay.none) {
willAttachRenderer();
willAttachRenderer();

if (renderer != null) {
// HTML element override attachTo method to attach renderObject to viewportBox.
if (parent is Element) {
RenderLayoutBox? parentRenderLayoutBox = parentElement?._renderLayoutBox?.renderScrollingContent ?? parentElement?._renderLayoutBox;
Expand Down Expand Up @@ -1706,9 +1707,9 @@ void _detachRenderBoxModel(RenderBox renderBox) {
RenderObject? parentRenderObject = renderBox.parent as RenderObject;

if (parentRenderObject is RenderObjectWithChildMixin) {
parentRenderObject.child = null; // RenderViewportBox
parentRenderObject.child = null; // Case for single child, eg. RenderViewportBox
} else if (parentRenderObject is ContainerRenderObjectMixin) {
parentRenderObject.remove(renderBox); // RenderLayoutBox or RenderSliverList
parentRenderObject.remove(renderBox); // Case for multi children, eg. RenderLayoutBox or RenderSliverList
}
}

Expand Down
18 changes: 12 additions & 6 deletions kraken/lib/src/dom/sliver_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,21 @@ class RenderSliverElementChildManager implements RenderSliverBoxChildManager {
throw FlutterError('Sliver unsupported type ${childNode.runtimeType} $childNode');
}

assert(child != null, 'Sliver render node should own RenderBox.');
// If renderer is not created, use an empty RenderBox to occupy the position, but not do layout or paint.
child ??= _createEmptyRenderObject();

if (_hasLayout) {
_sliverListLayout
..setupParentData(child!)
..insertSliverChild(child, after: after);
_sliverListLayout.insertSliverChild(child, after: after);
}

childNode.didAttachRenderer();
childNode.ensureChildAttached();
}

RenderBox _createEmptyRenderObject() {
return _RenderSliverItemProxy();
}

@override
bool debugAssertChildListLocked() => true;

Expand All @@ -98,8 +101,8 @@ class RenderSliverElementChildManager implements RenderSliverBoxChildManager {
}
}

// Fallback operation.
child.detach();
// Fallback operation, remove child from sliver list.
_sliverListLayout.remove(child);
}

@override
Expand Down Expand Up @@ -137,3 +140,6 @@ class RenderSliverElementChildManager implements RenderSliverBoxChildManager {
return trailingScrollOffset + averageExtent * remainingCount;
}
}

/// Used for the placeholder for empty sliver item.
class _RenderSliverItemProxy extends RenderProxyBox {}
13 changes: 5 additions & 8 deletions kraken/lib/src/rendering/box_model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,9 @@ mixin RenderBoxContainerDefaultsMixin<ChildType extends RenderBox,
/// walking the child list directly.
List<ChildType> getChildren() {
final List<ChildType> result = <ChildType>[];
RenderBox? child = firstChild;
while (child != null) {
final ParentDataType childParentData = child.parentData as ParentDataType;
visitChildren((child) {
result.add(child as ChildType);
child = childParentData.nextSibling;
}
});
return result;
}
}
Expand All @@ -162,7 +159,7 @@ class RenderLayoutBox extends RenderBoxModel
renderStyle: renderStyle,
);

// House content which can be scrolled.
// Host content which can be scrolled.
RenderLayoutBox? get renderScrollingContent {
if (firstChild is RenderLayoutBox) {
RenderLayoutBox _firstChild = firstChild as RenderLayoutBox;
Expand All @@ -176,8 +173,8 @@ class RenderLayoutBox extends RenderBoxModel
void markNeedsLayout() {
super.markNeedsLayout();

// FlexItem layout must trigger flex container to layout.
if (parent != null && parent is RenderFlexLayout) {
// FlexItem layout must trigger flex container to relayout.
if (parent is RenderFlexLayout) {
markParentNeedsLayout();
}
}
Expand Down