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

UIP-1802 CSS Transition Timeout/Warning #55

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
12 changes: 12 additions & 0 deletions lib/src/component/abstract_transition.dart
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ abstract class AbstractTransitionComponent<T extends AbstractTransitionProps, S
/// Whether the Element returned by [getTransitionDomNode] will have a transition event.
bool get hasTransition => true;

/// The duration that can elapse before a transition timeout occurs.
Duration get transitionTimeout => const Duration(seconds: 1);

// --------------------------------------------------------------------------
// Private Utility Methods
// --------------------------------------------------------------------------
Expand Down Expand Up @@ -184,7 +187,16 @@ abstract class AbstractTransitionComponent<T extends AbstractTransitionProps, S
skipCount = 0;
}

var timer = new Timer(transitionTimeout, () {
assert(ValidationUtil.warn(
'The number of transitions expected to complete have not completed. Something is most likely wrong.'
));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something else that I was thinking about. Should we still call complete if the timer completes? I could go either way, was just wondering what the desired behavior is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mm, yes, we should, in order to account for when the warning isn't caught by the time this is deployed


complete();
});

_endTransitionSubscription = getTransitionDomNode()?.onTransitionEnd?.skip(skipCount)?.take(1)?.listen((_) {
timer.cancel();
complete();
});
}
Expand Down
33 changes: 33 additions & 0 deletions test/over_react/component/abstract_transition_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,33 @@ main() {
});
}, testOn: '!js');
});

test('times out after the duration specified in timeoutDuration has elapsed', () async {
startRecordingValidationWarnings();

var renderedInstance = render(Transitioner()
..transitionCount = 1
..transitionTimeout = const Duration(seconds: 0)
);

TransitionerComponent transitioner = getDartComponent(renderedInstance);

expect(transitioner.state.transitionPhase, TransitionPhase.SHOWN);

transitioner.hide();

expect(transitioner.state.transitionPhase, TransitionPhase.HIDING);

await new Future.delayed(Duration.ZERO);

expect(transitioner.state.transitionPhase, TransitionPhase.HIDDEN);

verifyValidationWarning(
'The number of transitions expected to complete have not completed. Something is most likely wrong.'
);

stopRecordingValidationWarnings();
});
});
}

Expand All @@ -432,6 +459,8 @@ class TransitionerProps extends AbstractTransitionProps {

bool hasTransition;
bool initiallyShown;

Duration transitionTimeout;
}

@State()
Expand All @@ -444,6 +473,7 @@ class TransitionerComponent extends AbstractTransitionComponent<TransitionerProp
..addProps(super.getDefaultProps())
..hasTransition = true
..initiallyShown = true
..transitionTimeout = const Duration(seconds: 1)
);


Expand All @@ -456,6 +486,9 @@ class TransitionerComponent extends AbstractTransitionComponent<TransitionerProp
@override
bool get hasTransition => props.hasTransition;

@override
Duration get transitionTimeout => props.transitionTimeout;

@override
render() {
return Dom.div()();
Expand Down