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

Address CR feedback for SafeRenderManager #399

Merged
merged 1 commit into from
Oct 29, 2019
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
4 changes: 2 additions & 2 deletions lib/src/util/safe_render_manager/safe_render_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class SafeRenderManager extends Disposable {
SafeRenderManager({Element mountNode, this.autoAttachMountNode = false})
: mountNode = mountNode ?? new DivElement();

/// Renders [content]into [mountNode], chaining existing callback refs to
/// Renders [content] into [mountNode], chaining existing callback refs to
/// provide access to the rendered component via [contentRef].
void render(ReactElement content) {
_checkDisposalState();
Expand Down Expand Up @@ -103,7 +103,7 @@ class SafeRenderManager extends Disposable {
/// with whether the component was actually unmounted.
///
/// Unmounting could fail if a call to [render] is batched in with this
/// unmount during the propagation of this event. In that case, something
/// unmount during the propagation of this event. In that case, some
/// other call wanted something rendered and trumped the unmount request.
///
/// This behavior allows the same SafeRenderManager instance to be used to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,9 @@ class SafeRenderManagerHelperComponent extends UiStatefulComponent<SafeRenderMan

@override
render() {
final content = state.content;
if (content == null) return null;
if (state.content == null) return null;

return cloneElement(content, domProps()..ref = chainRef(content, _contentRef));
return cloneElement(state.content, domProps()..ref = chainRef(state.content, _contentRef));
}

void _contentRef(ref) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@TestOn('browser')
library safe_unmounter_test;
library safe_render_manager_helper_test;

import 'dart:async';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@TestOn('browser')
library top_level_render_manager_test;
library safe_render_manager_test;

import 'dart:async';
import 'dart:html';
Expand All @@ -14,7 +14,7 @@ import 'package:w_common/disposable.dart';

import 'test_component.dart';

/// Main entry point for TopLevelRenderManager testing
/// Main entry point for SafeRenderManager testing
main() {
setClientConfiguration();
enableTestMode();
Expand Down Expand Up @@ -72,7 +72,7 @@ main() {
expect(mountNode.children, isNotEmpty);
});

test('', () async {
test('', () {
renderManager.tryUnmount();
expect(mountNode.children, isEmpty);
});
Expand All @@ -95,7 +95,7 @@ main() {
});
});

test('invokes the provided callback immediately when nothing has been rendered', () async {
test('invokes the provided callback immediately when nothing has been rendered', () {
expect(mountNode.children, isEmpty, reason: 'test setup sanity check');

bool onUnmountCalledSynchronously = false;
Expand All @@ -109,7 +109,7 @@ main() {
});
});

group('automatically attaches and detached the mount node', () {
group('automatically attaches and detaches the mount node', () {
setUp(() async {
// Clean up the manager from the above setUp block.
await renderManager?.dispose();
Expand Down