From 699bd1315b1346681ea2c58be48e2c0b8fd2297e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Lu=C4=8Div=C5=88=C3=A1k?= Date: Tue, 10 Sep 2019 11:04:21 +0200 Subject: [PATCH 1/9] get rid of white flash --- packages/react/src/components/Embed/Embed.tsx | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/packages/react/src/components/Embed/Embed.tsx b/packages/react/src/components/Embed/Embed.tsx index 180961152e..bdab291e91 100644 --- a/packages/react/src/components/Embed/Embed.tsx +++ b/packages/react/src/components/Embed/Embed.tsx @@ -63,6 +63,7 @@ export interface EmbedProps extends UIComponentProps { export interface EmbedState { active: boolean + iframeLoaded: boolean } class Embed extends AutoControlledComponent, EmbedState> { @@ -104,7 +105,7 @@ class Embed extends AutoControlledComponent, EmbedState> } getInitialAutoControlledState(): EmbedState { - return { active: false } + return { active: false, iframeLoaded: false } } handleClick = e => { @@ -123,8 +124,17 @@ class Embed extends AutoControlledComponent, EmbedState> renderComponent({ ElementType, classes, accessibility, unhandledProps, styles, variables }) { const { control, iframe, placeholder, video } = this.props - const { active } = this.state + const { active, iframeLoaded } = this.state const controlVisible = !_.isNil(video) || !active + const placeholderImage = Image.create(placeholder, { + defaultProps: { + styles: styles.image, + variables: { + width: variables.width, + height: variables.height, + }, + }, + }) return ( , EmbedState> {...applyAccessibilityKeyHandlers(accessibility.keyHandlers.root, unhandledProps)} {...unhandledProps} > + {iframe && active && !iframeLoaded && placeholderImage} {active ? ( <> {Video.create(video, { @@ -142,6 +153,7 @@ class Embed extends AutoControlledComponent, EmbedState> controls: false, loop: true, muted: true, + poster: placeholder, styles: styles.video, variables: { width: variables.width, @@ -149,18 +161,19 @@ class Embed extends AutoControlledComponent, EmbedState> }, }, })} - {Box.create(iframe, { defaultProps: { as: 'iframe', styles: styles.iframe } })} + {Box.create(iframe, { + defaultProps: { + as: 'iframe', + styles: styles.iframe, + style: { visibility: iframeLoaded ? 'visible' : 'hidden' }, + onLoad: () => { + this.setState({ iframeLoaded: true }) + }, + }, + })} ) : ( - Image.create(placeholder, { - defaultProps: { - styles: styles.image, - variables: { - width: variables.width, - height: variables.height, - }, - }, - }) + placeholderImage )} {controlVisible && From 3523612f1f2af6c3fd270cb59fa424841f9cc389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Lu=C4=8Div=C5=88=C3=A1k?= Date: Tue, 10 Sep 2019 11:12:31 +0200 Subject: [PATCH 2/9] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a15cf55af5..cbfc0bac14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixes - Fix `Menu` and `Submenu` to use correct indicator icon and have correct width behavior [redlines] @bcalvery ([#1831](https://github.com/stardust-ui/react/pull/1831)) - Fix handling of `onMouseEnter` prop in `ChatMessage` @layershifter ([#1903](https://github.com/stardust-ui/react/pull/1903)) +- Fix white flash when activating `Embed` component @lucivpav ([#1909](https://github.com/stardust-ui/react/pull/1909)) ### Documentation - Fix broken code editor in some doc site examples and improve error experience @levithomason ([#1906](https://github.com/stardust-ui/react/pull/1906)) From 3badf5e6e23906791c3b072790c899bae667b75f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Lu=C4=8Div=C5=88=C3=A1k?= Date: Tue, 17 Sep 2019 17:52:39 +0200 Subject: [PATCH 3/9] make placeholder a string --- packages/react/src/components/Embed/Embed.tsx | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/react/src/components/Embed/Embed.tsx b/packages/react/src/components/Embed/Embed.tsx index bdab291e91..614e5f0d0a 100644 --- a/packages/react/src/components/Embed/Embed.tsx +++ b/packages/react/src/components/Embed/Embed.tsx @@ -14,7 +14,7 @@ import { import { embedBehavior } from '../../lib/accessibility' import { Accessibility } from '../../lib/accessibility/types' import Icon, { IconProps } from '../Icon/Icon' -import Image, { ImageProps } from '../Image/Image' +import Image from '../Image/Image' import Video, { VideoProps } from '../Video/Video' import Box, { BoxProps } from '../Box/Box' import { ComponentEventHandler, WithAsProp, ShorthandValue, withSafeTypeForAs } from '../../types' @@ -55,7 +55,7 @@ export interface EmbedProps extends UIComponentProps { onClick?: ComponentEventHandler /** Image source URL for when video isn't playing. */ - placeholder?: ShorthandValue + placeholder?: string /** Shorthand for an embedded video. */ video?: ShorthandValue @@ -126,15 +126,13 @@ class Embed extends AutoControlledComponent, EmbedState> const { control, iframe, placeholder, video } = this.props const { active, iframeLoaded } = this.state const controlVisible = !_.isNil(video) || !active - const placeholderImage = Image.create(placeholder, { - defaultProps: { - styles: styles.image, - variables: { - width: variables.width, - height: variables.height, - }, - }, - }) + const placeholderImage = placeholder ? ( + + ) : null return ( Date: Wed, 18 Sep 2019 10:50:29 +0200 Subject: [PATCH 4/9] move onLoad to overrideProps --- packages/react/src/components/Embed/Embed.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react/src/components/Embed/Embed.tsx b/packages/react/src/components/Embed/Embed.tsx index 614e5f0d0a..a6e622f3dc 100644 --- a/packages/react/src/components/Embed/Embed.tsx +++ b/packages/react/src/components/Embed/Embed.tsx @@ -164,6 +164,8 @@ class Embed extends AutoControlledComponent, EmbedState> as: 'iframe', styles: styles.iframe, style: { visibility: iframeLoaded ? 'visible' : 'hidden' }, + }, + overrideProps: { onLoad: () => { this.setState({ iframeLoaded: true }) }, From dde9df699ad8d32619681ea09c340671a5523dc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Lu=C4=8Div=C5=88=C3=A1k?= Date: Wed, 18 Sep 2019 15:23:32 +0200 Subject: [PATCH 5/9] extract to styles --- packages/react/src/components/Embed/Embed.tsx | 1 - .../react/src/themes/teams/components/Embed/embedStyles.ts | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/components/Embed/Embed.tsx b/packages/react/src/components/Embed/Embed.tsx index 7a218e0998..304afec8f2 100644 --- a/packages/react/src/components/Embed/Embed.tsx +++ b/packages/react/src/components/Embed/Embed.tsx @@ -163,7 +163,6 @@ class Embed extends AutoControlledComponent, EmbedState> defaultProps: { as: 'iframe', styles: styles.iframe, - style: { visibility: iframeLoaded ? 'visible' : 'hidden' }, }, overrideProps: { onLoad: () => { diff --git a/packages/react/src/themes/teams/components/Embed/embedStyles.ts b/packages/react/src/themes/teams/components/Embed/embedStyles.ts index 7f41874ab1..ee279fa590 100644 --- a/packages/react/src/themes/teams/components/Embed/embedStyles.ts +++ b/packages/react/src/themes/teams/components/Embed/embedStyles.ts @@ -48,7 +48,8 @@ export default { top: '50%', transform: 'translate(-50%, -50%)', }), - iframe: (): ICSSInJSStyle => ({ + iframe: ({ props: p }): ICSSInJSStyle => ({ display: 'block', + visibility: p.iframeLoaded ? 'visible' : 'hidden', }), } as ComponentSlotStylesPrepared From c6e3d83f69986bc0873506154b7df86180508b09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Lu=C4=8Div=C5=88=C3=A1k?= Date: Wed, 18 Sep 2019 17:06:32 +0200 Subject: [PATCH 6/9] use a handler in overrideProps --- packages/react/src/components/Embed/Embed.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/react/src/components/Embed/Embed.tsx b/packages/react/src/components/Embed/Embed.tsx index 304afec8f2..b52857bb0b 100644 --- a/packages/react/src/components/Embed/Embed.tsx +++ b/packages/react/src/components/Embed/Embed.tsx @@ -122,6 +122,13 @@ class Embed extends AutoControlledComponent, EmbedState> _.invoke(this.props, 'onClick', e, { ...this.props, active: !this.state.active }) } + handleFrameOverrides = predefinedProps => ({ + onLoad: (e: React.SyntheticEvent) => { + _.invoke(predefinedProps, 'onLoad', e) + this.setState({ iframeLoaded: true }) + }, + }) + renderComponent({ ElementType, classes, accessibility, unhandledProps, styles, variables }) { const { control, iframe, placeholder, video } = this.props const { active, iframeLoaded } = this.state @@ -164,11 +171,7 @@ class Embed extends AutoControlledComponent, EmbedState> as: 'iframe', styles: styles.iframe, }, - overrideProps: { - onLoad: () => { - this.setState({ iframeLoaded: true }) - }, - }, + overrideProps: this.handleFrameOverrides, })} ) : ( From 2643a1f4f98103989b9407c2261ea1e233863562 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Lu=C4=8Div=C5=88=C3=A1k?= Date: Wed, 18 Sep 2019 17:18:06 +0200 Subject: [PATCH 7/9] extract complex condition into a function --- packages/react/src/components/Embed/Embed.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/react/src/components/Embed/Embed.tsx b/packages/react/src/components/Embed/Embed.tsx index b52857bb0b..f3322ec10e 100644 --- a/packages/react/src/components/Embed/Embed.tsx +++ b/packages/react/src/components/Embed/Embed.tsx @@ -129,9 +129,17 @@ class Embed extends AutoControlledComponent, EmbedState> }, }) + renderIframePlaceholderImageIfNecessary(placeholderImage: JSX.Element) { + const { active, iframeLoaded } = this.state + if (active && !iframeLoaded) { + return placeholderImage + } + return null + } + renderComponent({ ElementType, classes, accessibility, unhandledProps, styles, variables }) { const { control, iframe, placeholder, video } = this.props - const { active, iframeLoaded } = this.state + const { active } = this.state const controlVisible = !_.isNil(video) || !active const placeholderImage = placeholder ? ( , EmbedState> {...unhandledProps} {...applyAccessibilityKeyHandlers(accessibility.keyHandlers.root, unhandledProps)} > - {iframe && active && !iframeLoaded && placeholderImage} + {iframe && this.renderIframePlaceholderImageIfNecessary(placeholderImage)} {active ? ( <> {Video.create(video, { From 129a2070e5d45500debf8ea005ac23ccda4babdf Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Thu, 19 Sep 2019 12:59:27 +0200 Subject: [PATCH 8/9] improvements --- packages/react/src/components/Embed/Embed.tsx | 56 +++++++++++-------- .../teams/components/Embed/embedStyles.ts | 2 +- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/packages/react/src/components/Embed/Embed.tsx b/packages/react/src/components/Embed/Embed.tsx index f3322ec10e..01ebe9c889 100644 --- a/packages/react/src/components/Embed/Embed.tsx +++ b/packages/react/src/components/Embed/Embed.tsx @@ -18,6 +18,7 @@ import Image from '../Image/Image' import Video, { VideoProps } from '../Video/Video' import Box, { BoxProps } from '../Box/Box' import { ComponentEventHandler, WithAsProp, ShorthandValue, withSafeTypeForAs } from '../../types' +import { Ref } from '@stardust-ui/react-component-ref' export interface EmbedSlotClassNames { control: string @@ -81,11 +82,17 @@ class Embed extends AutoControlledComponent, EmbedState> active: PropTypes.bool, defaultActive: PropTypes.bool, control: customPropTypes.itemShorthand, - iframe: customPropTypes.itemShorthand, + iframe: customPropTypes.every([ + customPropTypes.disallow(['video']), + customPropTypes.itemShorthand, + ]), onActiveChanged: PropTypes.func, onClick: PropTypes.func, placeholder: PropTypes.string, - video: customPropTypes.itemShorthand, + video: customPropTypes.every([ + customPropTypes.disallow(['iframe']), + customPropTypes.itemShorthand, + ]), } static defaultProps = { @@ -104,6 +111,8 @@ class Embed extends AutoControlledComponent, EmbedState> performClick: event => this.handleClick(event), } + frameRef = React.createRef() + getInitialAutoControlledState(): EmbedState { return { active: false, iframeLoaded: false } } @@ -125,23 +134,17 @@ class Embed extends AutoControlledComponent, EmbedState> handleFrameOverrides = predefinedProps => ({ onLoad: (e: React.SyntheticEvent) => { _.invoke(predefinedProps, 'onLoad', e) + this.setState({ iframeLoaded: true }) + this.frameRef.current.contentWindow.focus() }, }) - renderIframePlaceholderImageIfNecessary(placeholderImage: JSX.Element) { - const { active, iframeLoaded } = this.state - if (active && !iframeLoaded) { - return placeholderImage - } - return null - } - renderComponent({ ElementType, classes, accessibility, unhandledProps, styles, variables }) { const { control, iframe, placeholder, video } = this.props - const { active } = this.state - const controlVisible = !_.isNil(video) || !active - const placeholderImage = placeholder ? ( + const { active, iframeLoaded } = this.state + + const placeholderElement = placeholder ? ( , EmbedState> /> ) : null + const hasIframe = !_.isNil(iframe) + const hasVideo = !_.isNil(video) + const controlVisible = !active || hasVideo + const placeholderVisible = !active || (hasIframe && active && !iframeLoaded) + return ( , EmbedState> {...unhandledProps} {...applyAccessibilityKeyHandlers(accessibility.keyHandlers.root, unhandledProps)} > - {iframe && this.renderIframePlaceholderImageIfNecessary(placeholderImage)} - {active ? ( + {active && ( <> {Video.create(video, { defaultProps: { @@ -174,18 +181,19 @@ class Embed extends AutoControlledComponent, EmbedState> }, }, })} - {Box.create(iframe, { - defaultProps: { - as: 'iframe', - styles: styles.iframe, - }, - overrideProps: this.handleFrameOverrides, - })} + + {Box.create(iframe, { + defaultProps: { + as: 'iframe', + styles: styles.iframe, + }, + overrideProps: this.handleFrameOverrides, + })} + - ) : ( - placeholderImage )} + {placeholderVisible && placeholderElement} {controlVisible && Icon.create(control, { defaultProps: { diff --git a/packages/react/src/themes/teams/components/Embed/embedStyles.ts b/packages/react/src/themes/teams/components/Embed/embedStyles.ts index ee279fa590..3efff54bf4 100644 --- a/packages/react/src/themes/teams/components/Embed/embedStyles.ts +++ b/packages/react/src/themes/teams/components/Embed/embedStyles.ts @@ -50,6 +50,6 @@ export default { }), iframe: ({ props: p }): ICSSInJSStyle => ({ display: 'block', - visibility: p.iframeLoaded ? 'visible' : 'hidden', + ...(!p.iframeLoaded && { display: 'none' }), }), } as ComponentSlotStylesPrepared From 062ecc4759ecebe70f3890918c1df4e291b4666d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Lu=C4=8Div=C5=88=C3=A1k?= Date: Thu, 19 Sep 2019 14:48:19 +0200 Subject: [PATCH 9/9] fix test error --- packages/react/src/components/Embed/Embed.tsx | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/react/src/components/Embed/Embed.tsx b/packages/react/src/components/Embed/Embed.tsx index 01ebe9c889..9cfd5baf15 100644 --- a/packages/react/src/components/Embed/Embed.tsx +++ b/packages/react/src/components/Embed/Embed.tsx @@ -181,15 +181,17 @@ class Embed extends AutoControlledComponent, EmbedState> }, }, })} - - {Box.create(iframe, { - defaultProps: { - as: 'iframe', - styles: styles.iframe, - }, - overrideProps: this.handleFrameOverrides, - })} - + {iframe && ( + + {Box.create(iframe, { + defaultProps: { + as: 'iframe', + styles: styles.iframe, + }, + overrideProps: this.handleFrameOverrides, + })} + + )} )}