CPLAT-12205 Add WithTransition wrapper component to enable "controlled" transitions using props

[aaronlademann-wf]

Motivation

Currently, AbstractTransitionComponents can only be programmatically shown/hidden using the show / hide / toggle API methods on the instance.

With React 16 componentry - this is not ideal since calling the methods from a parent component requires the use of a ref, and the child component is no longer guaranteed to have been mounted when the parent component is. This makes it very difficult to hide/show things within data-driven applications without having to work around runtime race conditions.

Changes

1. Add the WithTransition wrapper component.
   This component is designed to have identical behavior to AbstractTransitionComponent, but allows the consumer to control the behavior using the value of props.isShown.
2. Add tests.

Release Notes

Add WithTransition wrapper component to enable "controlled" transitions using props

QA Checklist

• Tests were updated and provide good coverage of the changeset and other affected code

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

• A Client Platform member has reviewed these changes
• There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
• For release PRs - Version metadata in Rosie comment is correct

[aviary-wf]

Security Insights

No security relevant content was detected by automated scans.

Action Items

• Review PR for security impact; comment "security review required" if needed or unsure
• Verify aviary.yaml coverage of security relevant code

---

Questions or Comments? Reach out on Slack: #support-infosec.

[joebingham-wk]

This all looks awesome! I just have one question, but it's a small one, and then I'm +1!

[greglittlefield-wf]

Given the getDerivedStateFromProps issues and deprecation concerns, is it possible to add controlled behavior to this component cleanly?

We might be better off starting fresh with a new TransitionPrimitive component that's entirely props-based

[aaronlademann-wf]

@greglittlefield-wf can you be more specific with what you're seeing as far as breaking the rules of hooks?

All tests we used for the old component are passing, and we have a consumer +10.

[greglittlefield-wf]

@aaronlademann-wf Mostly around dependencies in useEffect/useLayoutEffect, and the complexity of the nested calls to render closures inside useLayoutEffect; see my last couple comments on the code

[greglittlefield-wf]

All tests we used for the old component are passing, and we have a consumer +10.

The current component has more edge cases and potentially incorrect code paths with the way it uses hooks, so I don't think that's enough to say the component in it's current state is bug-free.

Bugs introduced by improper uses of hooks might not always be surfaced in simple usages / tests, but rather by sequences of rerenders.

In this case, what about if you rerender the component when

[aaronlademann-wf]

@greglittlefield-wf I have addressed your feedback.

[maxwellpeterson-wf]

+10

• pulled into https://github.com/Workiva/wdesk_sdk/pull/5096 where there are two separate usages of WithTransition. Both work as expected.

[greglittlefield-wf]

@Workiva/release-management-p

[rmconsole-wf]

+1 from RM