From 7e47c633bfd71b7ec7633fc63c2f03115fa59b71 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 23 Aug 2022 10:08:20 -0400 Subject: [PATCH] Fix flakiness in unit test when using Signed-off-by: Sebastian Malton --- .../upgrade-chart-new-tab.test.ts.snap | 216 ------------------ .../upgrade-chart-new-tab.test.ts | 4 + src/renderer/components/animate/animate.tsx | 180 +++++++-------- .../default-enter-duration.injectable.ts | 12 + .../default-leave-duration.injectable.ts | 12 + .../request-animation-frame.injectable.ts | 5 +- src/renderer/components/menu/menu.tsx | 23 +- 7 files changed, 131 insertions(+), 321 deletions(-) create mode 100644 src/renderer/components/animate/default-enter-duration.injectable.ts create mode 100644 src/renderer/components/animate/default-leave-duration.injectable.ts diff --git a/src/features/helm-charts/upgrade-chart/__snapshots__/upgrade-chart-new-tab.test.ts.snap b/src/features/helm-charts/upgrade-chart/__snapshots__/upgrade-chart-new-tab.test.ts.snap index 3c5e4adce0..527f12724e 100644 --- a/src/features/helm-charts/upgrade-chart/__snapshots__/upgrade-chart-new-tab.test.ts.snap +++ b/src/features/helm-charts/upgrade-chart/__snapshots__/upgrade-chart-new-tab.test.ts.snap @@ -3116,60 +3116,6 @@ exports[`New Upgrade Helm Chart Dock Tab given a namespace is selected when navi - `; @@ -4167,60 +4113,6 @@ exports[`New Upgrade Helm Chart Dock Tab given a namespace is selected when navi - `; @@ -5220,60 +5112,6 @@ exports[`New Upgrade Helm Chart Dock Tab given a namespace is selected when navi - `; @@ -6041,59 +5879,5 @@ exports[`New Upgrade Helm Chart Dock Tab given a namespace is selected when navi - `; diff --git a/src/features/helm-charts/upgrade-chart/upgrade-chart-new-tab.test.ts b/src/features/helm-charts/upgrade-chart/upgrade-chart-new-tab.test.ts index bc3cbe53d8..c58cf24ba2 100644 --- a/src/features/helm-charts/upgrade-chart/upgrade-chart-new-tab.test.ts +++ b/src/features/helm-charts/upgrade-chart/upgrade-chart-new-tab.test.ts @@ -17,6 +17,7 @@ import type { RequestHelmReleaseConfiguration } from "../../../common/k8s-api/en import requestHelmReleaseConfigurationInjectable from "../../../common/k8s-api/endpoints/helm-releases.api/request-configuration.injectable"; import type { RequestHelmReleases } from "../../../common/k8s-api/endpoints/helm-releases.api/request-releases.injectable"; import requestHelmReleasesInjectable from "../../../common/k8s-api/endpoints/helm-releases.api/request-releases.injectable"; +import { advanceFakeTime, useFakeTime } from "../../../common/test-utils/use-fake-time"; import dockStoreInjectable from "../../../renderer/components/dock/dock/store.injectable"; import type { ApplicationBuilder } from "../../../renderer/components/test-utils/get-application-builder"; import { getApplicationBuilder } from "../../../renderer/components/test-utils/get-application-builder"; @@ -50,6 +51,8 @@ describe("New Upgrade Helm Chart Dock Tab", () => { navigateToHelmReleases = windowDi.inject(navigateToHelmReleasesInjectable); }); + useFakeTime("2020-01-12 12:00:00"); + builder.namespaces.add("my-first-namespace"); builder.namespaces.add("my-second-namespace"); @@ -114,6 +117,7 @@ describe("New Upgrade Helm Chart Dock Tab", () => { const upgradeHelmChartMenuItem = renderResult.getByTestId("upgrade-chart-menu-item-for-my-second-namespace/some-name"); upgradeHelmChartMenuItem.click(); + advanceFakeTime(100); }); it("renders", () => { diff --git a/src/renderer/components/animate/animate.tsx b/src/renderer/components/animate/animate.tsx index d642a41fa8..13ed615e33 100644 --- a/src/renderer/components/animate/animate.tsx +++ b/src/renderer/components/animate/animate.tsx @@ -4,12 +4,12 @@ */ import "./animate.scss"; -import React from "react"; -import { observable, makeObservable } from "mobx"; -import { observer } from "mobx-react"; +import React, { useEffect, useState } from "react"; import { cssNames, noop } from "../../utils"; import { withInjectables } from "@ogre-tools/injectable-react"; import requestAnimationFrameInjectable from "./request-animation-frame.injectable"; +import defaultEnterDurationForAnimatedInjectable from "./default-enter-duration.injectable"; +import defaultLeaveDurationForAnimatedInjectable from "./default-leave-duration.injectable"; export type AnimateName = "opacity" | "slide-right" | "opacity-scale" | string; @@ -25,114 +25,92 @@ export interface AnimateProps { interface Dependencies { requestAnimationFrame: (callback: () => void) => void; + defaultEnterDuration: number; + defaultLeaveDuration: number; } -@observer -class DefaultedAnimate extends React.Component { - static defaultProps = { - name: "opacity", - enter: true, - onEnter: noop, - onLeave: noop, - enterDuration: 100, - leaveDuration: 100, +const NonInjectedAnimate = (propsAndDeps: AnimateProps & Dependencies) => { + const { + requestAnimationFrame, + defaultEnterDuration, + defaultLeaveDuration, + ...props + } = propsAndDeps; + const { + children, + enter = true, + enterDuration = defaultEnterDuration, + leaveDuration = defaultLeaveDuration, + name = "opacity", + onEnter: onEnterHandler = noop<[]>, + onLeave: onLeaveHandler = noop<[]>, + } = props; + + const [isVisible, setIsVisible] = useState(enter); + const [showClassNameEnter, setShowClassNameEnter] = useState(false); + const [showClassNameLeave, setShowClassNameLeave] = useState(false); + + const contentElem = React.Children.only(children) as React.ReactElement>; + const onEnter = () => { + setIsVisible(true); + + requestAnimationFrame(() => { + setShowClassNameEnter(true); + onEnterHandler(); + }); }; + const onLeave = () => { + if (isVisible) { + setShowClassNameLeave(true); + onLeaveHandler(); - @observable isVisible = !!this.props.enter; - @observable statusClassName = { - enter: false, - leave: false, + // Cleanup after duration + setTimeout(() => { + setIsVisible(false); + setShowClassNameEnter(false); + setShowClassNameLeave(false); + }, leaveDuration); + } }; - - constructor(props: AnimateProps & Dependencies & typeof DefaultedAnimate.defaultProps) { - super(props); - makeObservable(this); - } - - get contentElem() { - return React.Children.only(this.props.children) as React.ReactElement>; - } - - private toggle(enter: boolean) { - if (enter) { - this.enter(); + const toggle = (entering: boolean) => { + if (entering) { + onEnter(); } else { - this.leave(); + onLeave(); } + }; + + useEffect(() => toggle(enter), [enter]); + + if (!isVisible) { + return null; } - componentDidMount() { - this.toggle(this.props.enter); - } + const cssVarsForAnimation = { + "--enter-duration": `${enterDuration}ms`, + "--leave-duration": `${leaveDuration}ms`, + } as React.CSSProperties; - componentDidUpdate(prevProps: Readonly): void { - const { enter } = this.props; - - if (prevProps.enter !== enter) { - this.toggle(enter); - } - } - - enter() { - this.isVisible = true; // triggers render() to apply css-animation in existing dom - - this.props.requestAnimationFrame(() => { - this.statusClassName.enter = true; - this.props.onEnter(); - }); - } - - leave() { - if (!this.isVisible) return; - this.statusClassName.leave = true; - this.props.onLeave(); - this.resetAfterLeaveDuration(); - } - - resetAfterLeaveDuration() { - setTimeout(() => this.reset(), this.props.leaveDuration); - } - - reset() { - this.isVisible = false; - this.statusClassName.enter = false; - this.statusClassName.leave = false; - } - - render() { - if (!this.isVisible) { - return null; - } - - const { name, enterDuration, leaveDuration } = this.props; - const contentElem = this.contentElem; - const cssVarsForAnimation = { - "--enter-duration": `${enterDuration}ms`, - "--leave-duration": `${leaveDuration}ms`, - } as React.CSSProperties; - - return React.cloneElement(contentElem, { - className: cssNames("Animate", name, contentElem.props.className, this.statusClassName), - children: contentElem.props.children, - style: { - ...contentElem.props.style, - ...cssVarsForAnimation, - }, - }); - } -} - -export const NonInjectedAnimate = (props: AnimateProps & Dependencies) => ; - -export const Animate = withInjectables( - NonInjectedAnimate, - - { - getProps: (di, props) => ({ - requestAnimationFrame: di.inject(requestAnimationFrameInjectable), - ...props, + return React.cloneElement(contentElem, { + className: cssNames("Animate", name, contentElem.props.className, { + enter: showClassNameEnter, + leave: showClassNameLeave, }), - }, -); + children: contentElem.props.children, + style: { + ...contentElem.props.style, + ...cssVarsForAnimation, + }, + }); +}; + +export const Animate = withInjectables(NonInjectedAnimate, { + getProps: (di, props) => ({ + ...props, + requestAnimationFrame: di.inject(requestAnimationFrameInjectable), + defaultEnterDuration: di.inject(defaultEnterDurationForAnimatedInjectable), + defaultLeaveDuration: di.inject(defaultLeaveDurationForAnimatedInjectable), + }), +}); diff --git a/src/renderer/components/animate/default-enter-duration.injectable.ts b/src/renderer/components/animate/default-enter-duration.injectable.ts new file mode 100644 index 0000000000..c28f8a3346 --- /dev/null +++ b/src/renderer/components/animate/default-enter-duration.injectable.ts @@ -0,0 +1,12 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; + +const defaultEnterDurationForAnimatedInjectable = getInjectable({ + id: "default-enter-duration-for-animated", + instantiate: () => 100, +}); + +export default defaultEnterDurationForAnimatedInjectable; diff --git a/src/renderer/components/animate/default-leave-duration.injectable.ts b/src/renderer/components/animate/default-leave-duration.injectable.ts new file mode 100644 index 0000000000..01193151ab --- /dev/null +++ b/src/renderer/components/animate/default-leave-duration.injectable.ts @@ -0,0 +1,12 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; + +const defaultLeaveDurationForAnimatedInjectable = getInjectable({ + id: "default-leave-duration-for-animated", + instantiate: () => 100, +}); + +export default defaultLeaveDurationForAnimatedInjectable; diff --git a/src/renderer/components/animate/request-animation-frame.injectable.ts b/src/renderer/components/animate/request-animation-frame.injectable.ts index f61b2ae874..c04fa3a72e 100644 --- a/src/renderer/components/animate/request-animation-frame.injectable.ts +++ b/src/renderer/components/animate/request-animation-frame.injectable.ts @@ -4,9 +4,12 @@ */ import { getInjectable } from "@ogre-tools/injectable"; +export type RequestAnimationFrame = (callback: () => void) => void; + const requestAnimationFrameInjectable = getInjectable({ id: "request-animation-frame", - instantiate: () => (callback: () => void) => requestAnimationFrame(callback), + // NOTE: this cannot be simplified to just `=> requestAnimationFrame` else an Illegal Invocation error will be thrown + instantiate: (): RequestAnimationFrame => (callback) => requestAnimationFrame(callback), causesSideEffects: true, }); diff --git a/src/renderer/components/menu/menu.tsx b/src/renderer/components/menu/menu.tsx index 582ad37af4..b43f3a497b 100644 --- a/src/renderer/components/menu/menu.tsx +++ b/src/renderer/components/menu/menu.tsx @@ -13,9 +13,15 @@ import { Animate } from "../animate"; import type { IconProps } from "../icon"; import { Icon } from "../icon"; import isEqual from "lodash/isEqual"; +import type { RequestAnimationFrame } from "../animate/request-animation-frame.injectable"; +import { withInjectables } from "@ogre-tools/injectable-react"; +import requestAnimationFrameInjectable from "../animate/request-animation-frame.injectable"; export const MenuContext = React.createContext(null); -export type MenuContextValue = Menu; +export interface MenuContextValue { + readonly props: Readonly; + close: () => void; +} export interface MenuPosition { left?: boolean; @@ -62,10 +68,14 @@ const defaultPropsMenu: Partial = { animated: true, }; -export class Menu extends React.Component { +interface Dependencies { + requestAnimationFrame: RequestAnimationFrame; +} + +class NonInjectedMenu extends React.Component { static defaultProps = defaultPropsMenu as object; - constructor(props: MenuProps) { + constructor(props: MenuProps & Dependencies) { super(props); autoBind(this); } @@ -372,6 +382,13 @@ export class Menu extends React.Component { } } +export const Menu = withInjectables(NonInjectedMenu, { + getProps: (di, props) => ({ + ...props, + requestAnimationFrame: di.inject(requestAnimationFrameInjectable), + }), +}); + export function SubMenu(props: Partial) { const { className, ...menuProps } = props;