From ee8516afb5ee82ab4898e964e67331366202d93d Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Sun, 16 Jul 2023 17:39:51 -0300 Subject: [PATCH 01/20] fix: LSDV-5256: Stop automatically scrolling if current Paragraph is not in the screen --- e2e/tests/sync/audio-paragraphs.test.js | 53 ++++++++++++++++++++ src/tags/object/Paragraphs/HtxParagraphs.js | 54 ++++++++++++++++----- 2 files changed, 96 insertions(+), 11 deletions(-) diff --git a/e2e/tests/sync/audio-paragraphs.test.js b/e2e/tests/sync/audio-paragraphs.test.js index ef4e10d23..958e2841a 100644 --- a/e2e/tests/sync/audio-paragraphs.test.js +++ b/e2e/tests/sync/audio-paragraphs.test.js @@ -352,5 +352,58 @@ FFlagMatrix(['fflag_feat_front_lsdv_e_278_contextual_scrolling_short'], function await assert.equal(scrollPosition.scrollTop, 0); }); + + FFlagScenario('Paragraph shouldnt automatically scroll if user manually scroll and the current paragraph is not in the screen', async function({ I, LabelStudio, AtAudioView }) { + LabelStudio.setFeatureFlags({ + ff_front_dev_2715_audio_3_280722_short: true, + ff_front_1170_outliner_030222_short: true, + ...flags, + }); + + params.config = configWithScroll; + + I.amOnPage('/'); + + LabelStudio.init(params); + + await AtAudioView.waitForAudio(); + await AtAudioView.lookForStage(); + + const [{ currentTime: startingAudioTime }, { currentTime: startingParagraphAudioTime }] = await AtAudioView.getCurrentAudio(); + + assert.equal(startingAudioTime, startingParagraphAudioTime); + assert.equal(startingParagraphAudioTime, 0); + + AtAudioView.clickPlayButton(); + + I.wait(2); + + I.executeScript( () => { + document.querySelector('[data-testid="phrases-wrapper"]').scrollTo(0, 1000); + + const wheelEvt = document.createEvent('MouseEvents'); + + wheelEvt.initEvent('wheel', true, true); + + wheelEvt.deltaY = 1200; + + document.querySelector('[data-testid="phrases-wrapper"]').dispatchEvent(wheelEvt); + }); + + I.wait(5); + + const scrollPosition = await I.executeScript(function(selector) { + const element = document.querySelector(selector); + + return { + scrollTop: element.scrollTop, + scrollLeft: element.scrollLeft, + }; + }, '[data-testid="phrases-wrapper"]'); + + console.log(scrollPosition.scrollTop); + + await assert(scrollPosition.scrollTop > 400, 'Scroll position should be greater than 200'); + }); } }); diff --git a/src/tags/object/Paragraphs/HtxParagraphs.js b/src/tags/object/Paragraphs/HtxParagraphs.js index ec67a8496..bc5bd780c 100644 --- a/src/tags/object/Paragraphs/HtxParagraphs.js +++ b/src/tags/object/Paragraphs/HtxParagraphs.js @@ -28,6 +28,7 @@ class HtxParagraphsView extends Component { this.isPlaying = false; this.state = { canScroll: true, + inViewport: true, }; } @@ -427,32 +428,49 @@ class HtxParagraphsView extends Component { const _wrapperOffsetTop = this.activeRef.current?.offsetTop - _padding; const _splittedText = 10; // it will be from 0 to 100% of the text height, going 10% by 10% + this._handleScrollRoot(); this._disposeTimeout(); if (_phaseHeight > _wrapperHeight) { for (let i = 0; i < _splittedText; i++) { this.scrollTimeout.push( setTimeout(() => { - const _pos = (_wrapperOffsetTop) + ((_phaseHeight - (_wrapperHeight / 3)) * (i * .1)); // 1/3 of the wrapper height is the offset to keep the text aligned with the middle of the wrapper + const _pos = (_wrapperOffsetTop) + ((_phaseHeight - (_wrapperHeight / 3)) * (i * (1 / _splittedText))); // 1/3 of the wrapper height is the offset to keep the text aligned with the middle of the wrapper - root.scrollTo({ - top: _pos, - behavior: 'smooth', - }); + if (this.state.inViewPort && this.state.canScroll) { + root.scrollTo({ + top: _pos, + behavior: 'smooth', + }); + } }, ((_duration / _splittedText) * i) * 1000), ); } } else { - root.scrollTo({ - top: _wrapperOffsetTop, - behavior: 'smooth', - }); + if (this.state.inViewPort) { + root.scrollTo({ + top: _wrapperOffsetTop, + behavior: 'smooth', + }); + } } this.lastPlayingId = item.playingId; } } + _handleScrollToPhrase() { + this.setState( { inViewPort : true }); + + const _padding = 8; // 8 is the padding between the phrases, so it will keep aligned with the top of the phrase + const _wrapperOffsetTop = this.activeRef.current?.offsetTop - _padding; + + this.myRef.current.scrollTo({ + top: _wrapperOffsetTop, + behavior: 'smooth', + }); + } + _handleScrollContainerHeight() { const container = this.myRef.current; const mainContentView = document.querySelector('.lsf-main-content'); @@ -471,7 +489,16 @@ class HtxParagraphsView extends Component { } _handleScrollRoot() { - this._disposeTimeout(); + if (this.activeRef.current) { + const { top, bottom, height } = this.activeRef.current.getBoundingClientRect(); + const { offsetHeight, offsetTop } = this.myRef.current; + const offset = offsetTop + 95; + const off = height > offsetHeight ? height - offsetHeight : 0; + const isInView = ((top > offset && top < offsetHeight + offset) || + (bottom > offset && bottom < offsetHeight + offset + off)); + + this.setState({ inViewPort: isInView }); + } } _resizeObserver = new ResizeObserver(() => this._handleScrollContainerHeight()); @@ -511,7 +538,12 @@ class HtxParagraphsView extends Component { data-testid={'auto-scroll-toggle'} checked={this.state.canScroll} onChange={() => { - this.setState({ canScroll: !this.state.canScroll }); + if (!this.state.canScroll) + this._handleScrollToPhrase(); + + this.setState({ + canScroll: !this.state.canScroll, + }); }} label={'Auto-scroll'} /> From 578b8f88c883a70778719a9667b89fae6d34a0cf Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Sun, 16 Jul 2023 17:44:06 -0300 Subject: [PATCH 02/20] remove leftover console log --- e2e/tests/sync/audio-paragraphs.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/e2e/tests/sync/audio-paragraphs.test.js b/e2e/tests/sync/audio-paragraphs.test.js index 958e2841a..32db4cebc 100644 --- a/e2e/tests/sync/audio-paragraphs.test.js +++ b/e2e/tests/sync/audio-paragraphs.test.js @@ -401,8 +401,6 @@ FFlagMatrix(['fflag_feat_front_lsdv_e_278_contextual_scrolling_short'], function }; }, '[data-testid="phrases-wrapper"]'); - console.log(scrollPosition.scrollTop); - await assert(scrollPosition.scrollTop > 400, 'Scroll position should be greater than 200'); }); } From bc154b3712047b4f5675da4e8a1cb4d4fadbd6b5 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Mon, 17 Jul 2023 16:25:29 -0300 Subject: [PATCH 03/20] start get the padding element and stop using magic numbers --- src/tags/object/Paragraphs/HtxParagraphs.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/tags/object/Paragraphs/HtxParagraphs.js b/src/tags/object/Paragraphs/HtxParagraphs.js index bc5bd780c..55b532c04 100644 --- a/src/tags/object/Paragraphs/HtxParagraphs.js +++ b/src/tags/object/Paragraphs/HtxParagraphs.js @@ -418,7 +418,7 @@ class HtxParagraphsView extends Component { if (isFF(FF_LSDV_E_278) && this.props.item.contextscroll && item.playingId >= 0 && this.lastPlayingId !== item.playingId && this.state.canScroll) { - const _padding = 8; // 8 is the padding between the phrases, so it will keep aligned with the top of the phrase + const _padding = parseInt(window.getComputedStyle(this.myRef.current)?.getPropertyValue('padding-top')) || 0; const _playingItem = this.props.item._value[item.playingId]; const _start = _playingItem.start; const _end = _playingItem.end; @@ -428,6 +428,8 @@ class HtxParagraphsView extends Component { const _wrapperOffsetTop = this.activeRef.current?.offsetTop - _padding; const _splittedText = 10; // it will be from 0 to 100% of the text height, going 10% by 10% + console.log('padding', _padding); + this._handleScrollRoot(); this._disposeTimeout(); @@ -462,7 +464,7 @@ class HtxParagraphsView extends Component { _handleScrollToPhrase() { this.setState( { inViewPort : true }); - const _padding = 8; // 8 is the padding between the phrases, so it will keep aligned with the top of the phrase + const _padding = parseInt(window.getComputedStyle(this.myRef.current)?.getPropertyValue('padding-top')) || 0; // 8 is the padding between the phrases, so it will keep aligned with the top of the phrase const _wrapperOffsetTop = this.activeRef.current?.offsetTop - _padding; this.myRef.current.scrollTo({ @@ -491,11 +493,11 @@ class HtxParagraphsView extends Component { _handleScrollRoot() { if (this.activeRef.current) { const { top, bottom, height } = this.activeRef.current.getBoundingClientRect(); - const { offsetHeight, offsetTop } = this.myRef.current; - const offset = offsetTop + 95; - const off = height > offsetHeight ? height - offsetHeight : 0; + const { offsetHeight } = this.myRef.current; + const offset = this.myRef.current.getBoundingClientRect().top; + const heightDifference = height > offsetHeight ? height - offsetHeight : 0; const isInView = ((top > offset && top < offsetHeight + offset) || - (bottom > offset && bottom < offsetHeight + offset + off)); + (bottom > offset && bottom < offsetHeight + offset + heightDifference)); this.setState({ inViewPort: isInView }); } From 6f876ffd88c1012854e054c0a6bfef9bd2cd32c4 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Mon, 17 Jul 2023 19:20:05 -0300 Subject: [PATCH 04/20] remove leftover console log --- src/tags/object/Paragraphs/HtxParagraphs.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tags/object/Paragraphs/HtxParagraphs.js b/src/tags/object/Paragraphs/HtxParagraphs.js index 55b532c04..ce4d238b3 100644 --- a/src/tags/object/Paragraphs/HtxParagraphs.js +++ b/src/tags/object/Paragraphs/HtxParagraphs.js @@ -428,8 +428,6 @@ class HtxParagraphsView extends Component { const _wrapperOffsetTop = this.activeRef.current?.offsetTop - _padding; const _splittedText = 10; // it will be from 0 to 100% of the text height, going 10% by 10% - console.log('padding', _padding); - this._handleScrollRoot(); this._disposeTimeout(); From e27acc65de3006a73bc3d2b1d97b277e254b5961 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Thu, 20 Jul 2023 17:10:57 -0300 Subject: [PATCH 05/20] fix audio seek sync with scroll --- src/tags/object/Paragraphs/HtxParagraphs.js | 28 +---- .../object/Paragraphs/Paragraphs.module.scss | 13 ++ src/tags/object/Paragraphs/Phrases.js | 112 +++++++++++++++++- 3 files changed, 128 insertions(+), 25 deletions(-) diff --git a/src/tags/object/Paragraphs/HtxParagraphs.js b/src/tags/object/Paragraphs/HtxParagraphs.js index ce4d238b3..2692c7c59 100644 --- a/src/tags/object/Paragraphs/HtxParagraphs.js +++ b/src/tags/object/Paragraphs/HtxParagraphs.js @@ -428,7 +428,6 @@ class HtxParagraphsView extends Component { const _wrapperOffsetTop = this.activeRef.current?.offsetTop - _padding; const _splittedText = 10; // it will be from 0 to 100% of the text height, going 10% by 10% - this._handleScrollRoot(); this._disposeTimeout(); if (_phaseHeight > _wrapperHeight) { @@ -460,8 +459,6 @@ class HtxParagraphsView extends Component { } _handleScrollToPhrase() { - this.setState( { inViewPort : true }); - const _padding = parseInt(window.getComputedStyle(this.myRef.current)?.getPropertyValue('padding-top')) || 0; // 8 is the padding between the phrases, so it will keep aligned with the top of the phrase const _wrapperOffsetTop = this.activeRef.current?.offsetTop - _padding; @@ -488,19 +485,6 @@ class HtxParagraphsView extends Component { } - _handleScrollRoot() { - if (this.activeRef.current) { - const { top, bottom, height } = this.activeRef.current.getBoundingClientRect(); - const { offsetHeight } = this.myRef.current; - const offset = this.myRef.current.getBoundingClientRect().top; - const heightDifference = height > offsetHeight ? height - offsetHeight : 0; - const isInView = ((top > offset && top < offsetHeight + offset) || - (bottom > offset && bottom < offsetHeight + offset + heightDifference)); - - this.setState({ inViewPort: isInView }); - } - } - _resizeObserver = new ResizeObserver(() => this._handleScrollContainerHeight()); componentDidUpdate() { @@ -510,21 +494,19 @@ class HtxParagraphsView extends Component { componentDidMount() { if(isFF(FF_LSDV_E_278) && this.props.item.contextscroll) this._resizeObserver.observe(document.querySelector('.lsf-main-content')); this._handleUpdate(); - - if(isFF(FF_LSDV_E_278)) - this.myRef.current.addEventListener('wheel', this._handleScrollRoot.bind(this)); } componentWillUnmount() { const target = document.querySelector('.lsf-main-content'); - if(isFF(FF_LSDV_E_278)) - this.myRef.current.removeEventListener('wheel', this._handleScrollRoot); - if (target) this._resizeObserver?.unobserve(target); this._resizeObserver?.disconnect(); } + setIsInViewPort(isInViewPort) { + this.setState({ inViewPort: isInViewPort }); + } + renderWrapperHeader() { const { item } = this.props; @@ -592,7 +574,7 @@ class HtxParagraphsView extends Component { className={contextScroll ? styles.scroll_container : styles.container} onMouseUp={this.onMouseUp.bind(this)} > - + ); diff --git a/src/tags/object/Paragraphs/Paragraphs.module.scss b/src/tags/object/Paragraphs/Paragraphs.module.scss index b8925429c..7f173e291 100644 --- a/src/tags/object/Paragraphs/Paragraphs.module.scss +++ b/src/tags/object/Paragraphs/Paragraphs.module.scss @@ -257,4 +257,17 @@ $border-thin: 1px solid rgba(137, 128, 152, 0.16); color: #898098; } } + + .text { + position: relative; + + .readingLine { + position: absolute; + bottom: 0; + left: 0; + width: 100%; + height: 20px; + pointer-events: none; + } + } } diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index bd9e1a8f7..66696d97b 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -5,6 +5,7 @@ import { PauseCircleOutlined, PlayCircleOutlined } from '@ant-design/icons'; import styles from './Paragraphs.module.scss'; import { FF_LSDV_E_278, isFF } from '../../../utils/feature-flags'; import { IconPause, IconPlay } from '../../../assets/icons'; +import { useCallback, useEffect, useState } from 'react'; const formatTime = (seconds) => { if (isNaN(seconds)) return ''; @@ -20,9 +21,111 @@ const formatTime = (seconds) => { return `${formattedHours}:${formattedMinutes}:${formattedSeconds}`; }; -export const Phrases = observer(({ item, playingId, activeRef }) => { +export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport }) => { + const [animationKeyFrame, setAnimationKeyFrame] = useState(null); + const [seek, setSeek] = useState(0); + const [isSeek, setIsSeek] = useState(null); const cls = item.layoutClasses; const withAudio = !!item.audio; + let observer; + + // default function to animate the reading line + const animateElement = useCallback( + (element, start, duration, isPlaying = true) => { + if (!element || !isFF(FF_LSDV_E_278)) return; + + const _animationKeyFrame = element.animate( + [{ top: `${start}%` }, { top: '100%' }], + { + easing: 'linear', + duration: duration * 1000, + }, + ); + + if (isPlaying) + _animationKeyFrame.play(); + else + _animationKeyFrame.pause(); + + setAnimationKeyFrame(_animationKeyFrame); + }, + [animationKeyFrame, setAnimationKeyFrame], + ); + + // this function is used to animate the reading line when user seek audio + const setSeekAnimation = useCallback( + (isSeeking) => { + if (!isFF(FF_LSDV_E_278)) return; + + const duration = item._value[playingId]?.duration || item._value[playingId]?.end - item._value[playingId]?.start; + const endTime = !item._value[playingId]?.end ? item._value[playingId]?.start + item._value[playingId]?.duration : item._value[playingId]?.end; + const seekDuration = endTime - seek.time; + const startValue = 100 - ((seekDuration * 100) / duration); + + if (startValue > 0 && startValue < 100) + animateElement(activeRef.current?.querySelector('.reading-line'), startValue, seekDuration, seek.playing); + else + setIsSeek(isSeeking); + }, + [seek, playingId], + ); + + // useRef to get the reading line element + const readingLineRef = useCallback(node => { + if(observer) { + observer.disconnect(); + } + + if(node !== null) { + const duration = item._value[playingId]?.duration || item._value[playingId]?.end - item._value[playingId]?.start; + + if (!isNaN(duration)) { + animateElement(node, 0, duration, item.playing); + } + + observer = new IntersectionObserver((entries) => { + setIsInViewport(entries[0].isIntersecting); + }, { + rootMargin: '0px', + }); + + observer.observe(node); + } + }, [playingId]); + + useEffect(() => { + if (!isFF(FF_LSDV_E_278)) return; + + item.syncHandlers.set('seek', seek => { + item.handleSyncPlay(seek); + setSeek(seek); + }); + + return () => { + observer?.disconnect(); + }; + }, []); + + + // when user seek audio, the useEffect will be triggered and animate the reading line to the seek position + useEffect(() => { + setSeekAnimation(true); + }, [seek]); + + // when user seek audio to a differente playing phrase, the useEffect will be triggered and animate the reading line to the seek position + useEffect(() => { + if (!isSeek) return; + + setSeekAnimation(false); + }, [playingId]); + + // when user click on play/pause button, the useEffect will be triggered and pause or play the reading line animation + useEffect(() => { + if (!isFF(FF_LSDV_E_278)) return; + + if (item.playing) animationKeyFrame?.play(); + else animationKeyFrame?.pause(); + }, [item.playing]); if (!item._value) return null; @@ -69,7 +172,12 @@ export const Phrases = observer(({ item, playingId, activeRef }) => { {v[item.namekey]} )} - {v[item.textkey]} + + {(isFF(FF_LSDV_E_278) && isActive) && ( + + )} + {v[item.textkey]} + ); }); From ace2f80c9fd3df690e33f5d55c905358a44bc353 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Fri, 21 Jul 2023 12:33:40 -0300 Subject: [PATCH 06/20] small fixies about scroll position and scroll validation --- src/tags/object/Paragraphs/HtxParagraphs.js | 2 +- src/tags/object/Paragraphs/Phrases.js | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/tags/object/Paragraphs/HtxParagraphs.js b/src/tags/object/Paragraphs/HtxParagraphs.js index 2692c7c59..ff8a4982c 100644 --- a/src/tags/object/Paragraphs/HtxParagraphs.js +++ b/src/tags/object/Paragraphs/HtxParagraphs.js @@ -426,7 +426,7 @@ class HtxParagraphsView extends Component { const _duration = this.props.item._value[item.playingId].duration || _end - _start; const _wrapperHeight = root.offsetHeight; const _wrapperOffsetTop = this.activeRef.current?.offsetTop - _padding; - const _splittedText = 10; // it will be from 0 to 100% of the text height, going 10% by 10% + const _splittedText = Math.ceil(this.activeRef.current?.offsetHeight / this.myRef.current?.offsetHeight) + 1; // +1 to make sure the last line is scrolled to the top this._disposeTimeout(); diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index 66696d97b..684ca4adf 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -99,6 +99,7 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } item.syncHandlers.set('seek', seek => { item.handleSyncPlay(seek); setSeek(seek); + setIsInViewport(true); }); return () => { @@ -112,7 +113,7 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } setSeekAnimation(true); }, [seek]); - // when user seek audio to a differente playing phrase, the useEffect will be triggered and animate the reading line to the seek position + // when user seek audio to a different playing phrase, the useEffect will be triggered and animate the reading line to the seek position useEffect(() => { if (!isSeek) return; @@ -160,7 +161,10 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } isFF(FF_LSDV_E_278) ? : } - onClick={() => item.play(idx)} + onClick={() => { + setIsInViewport(true); + item.play(idx); + }} /> )} {isFF(FF_LSDV_E_278) ? ( From 75bb1be1003ed7ffcbf2cd648606160e62cf14e6 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Mon, 24 Jul 2023 12:24:17 -0300 Subject: [PATCH 07/20] fix regression with auto scroll if Phrase is annotated --- src/tags/object/Paragraphs/Phrases.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index 684ca4adf..55783de85 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -177,10 +177,10 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } )} + {v[item.textkey]} {(isFF(FF_LSDV_E_278) && isActive) && ( )} - {v[item.textkey]} ); From e458e5a58d87702940e7714eba62fd6a58318d49 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Mon, 24 Jul 2023 15:34:08 -0300 Subject: [PATCH 08/20] fix the auto scroll position --- src/tags/object/Paragraphs/HtxParagraphs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tags/object/Paragraphs/HtxParagraphs.js b/src/tags/object/Paragraphs/HtxParagraphs.js index ff8a4982c..6ef8e02ce 100644 --- a/src/tags/object/Paragraphs/HtxParagraphs.js +++ b/src/tags/object/Paragraphs/HtxParagraphs.js @@ -434,7 +434,7 @@ class HtxParagraphsView extends Component { for (let i = 0; i < _splittedText; i++) { this.scrollTimeout.push( setTimeout(() => { - const _pos = (_wrapperOffsetTop) + ((_phaseHeight - (_wrapperHeight / 3)) * (i * (1 / _splittedText))); // 1/3 of the wrapper height is the offset to keep the text aligned with the middle of the wrapper + const _pos = (_wrapperOffsetTop) + ((_phaseHeight) * (i * (1 / _splittedText))); if (this.state.inViewPort && this.state.canScroll) { root.scrollTo({ From 180e5005284db9bddd8b62d7204243676bc24d30 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Mon, 24 Jul 2023 15:58:14 -0300 Subject: [PATCH 09/20] remove unecessary comment --- src/tags/object/Paragraphs/HtxParagraphs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tags/object/Paragraphs/HtxParagraphs.js b/src/tags/object/Paragraphs/HtxParagraphs.js index 6ef8e02ce..bda15c409 100644 --- a/src/tags/object/Paragraphs/HtxParagraphs.js +++ b/src/tags/object/Paragraphs/HtxParagraphs.js @@ -459,7 +459,7 @@ class HtxParagraphsView extends Component { } _handleScrollToPhrase() { - const _padding = parseInt(window.getComputedStyle(this.myRef.current)?.getPropertyValue('padding-top')) || 0; // 8 is the padding between the phrases, so it will keep aligned with the top of the phrase + const _padding = parseInt(window.getComputedStyle(this.myRef.current)?.getPropertyValue('padding-top')) || 0; const _wrapperOffsetTop = this.activeRef.current?.offsetTop - _padding; this.myRef.current.scrollTo({ From 3c07f88d5659af1592b4e958860bc1d7e1cdcb24 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Mon, 24 Jul 2023 16:27:22 -0300 Subject: [PATCH 10/20] changing the scroll position to stop breaking the UI --- .../object/Paragraphs/Paragraphs.module.scss | 19 +++++++++++-------- src/tags/object/Paragraphs/Phrases.js | 7 ++++--- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/tags/object/Paragraphs/Paragraphs.module.scss b/src/tags/object/Paragraphs/Paragraphs.module.scss index 7f173e291..5be4f7f66 100644 --- a/src/tags/object/Paragraphs/Paragraphs.module.scss +++ b/src/tags/object/Paragraphs/Paragraphs.module.scss @@ -204,6 +204,7 @@ $border-thin: 1px solid rgba(137, 128, 152, 0.16); display: flex; flex-wrap: wrap; width: calc(100% - 36px); + position: relative; &.collapsed { background-color: var(--highlight-color); @@ -260,14 +261,16 @@ $border-thin: 1px solid rgba(137, 128, 152, 0.16); .text { position: relative; + } - .readingLine { - position: absolute; - bottom: 0; - left: 0; - width: 100%; - height: 20px; - pointer-events: none; - } + .readingLine { + position: absolute; + bottom: 0; + left: 0; + width: 100%; + height: 20px; + pointer-events: none; + background: blue; + opacity: .2; } } diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index 8aced0b11..b60ca2539 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -175,11 +175,12 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } {v[item.namekey]} )} + {(isFF(FF_LSDV_E_278) && isActive) && ( + + )} + {v[item.textkey]} - {(isFF(FF_LSDV_E_278) && isActive) && ( - - )} ); From 3c8e1f296de948fad32c2ac1d062bddc2b3b54ec Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Mon, 24 Jul 2023 16:39:28 -0300 Subject: [PATCH 11/20] roll back the changes --- .../object/Paragraphs/Paragraphs.module.scss | 19 ++++++++----------- src/tags/object/Paragraphs/Phrases.js | 7 +++---- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/tags/object/Paragraphs/Paragraphs.module.scss b/src/tags/object/Paragraphs/Paragraphs.module.scss index 5be4f7f66..7f173e291 100644 --- a/src/tags/object/Paragraphs/Paragraphs.module.scss +++ b/src/tags/object/Paragraphs/Paragraphs.module.scss @@ -204,7 +204,6 @@ $border-thin: 1px solid rgba(137, 128, 152, 0.16); display: flex; flex-wrap: wrap; width: calc(100% - 36px); - position: relative; &.collapsed { background-color: var(--highlight-color); @@ -261,16 +260,14 @@ $border-thin: 1px solid rgba(137, 128, 152, 0.16); .text { position: relative; - } - .readingLine { - position: absolute; - bottom: 0; - left: 0; - width: 100%; - height: 20px; - pointer-events: none; - background: blue; - opacity: .2; + .readingLine { + position: absolute; + bottom: 0; + left: 0; + width: 100%; + height: 20px; + pointer-events: none; + } } } diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index b60ca2539..8aced0b11 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -175,12 +175,11 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } {v[item.namekey]} )} - {(isFF(FF_LSDV_E_278) && isActive) && ( - - )} - {v[item.textkey]} + {(isFF(FF_LSDV_E_278) && isActive) && ( + + )} ); From 6dd18329d94653830940b17408908e70f5884bf8 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Mon, 24 Jul 2023 17:13:52 -0300 Subject: [PATCH 12/20] fix the way that the reading line is rendering to influence the Paragraph parser --- .../object/Paragraphs/Paragraphs.module.scss | 2 +- src/tags/object/Paragraphs/Phrases.js | 20 +++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/tags/object/Paragraphs/Paragraphs.module.scss b/src/tags/object/Paragraphs/Paragraphs.module.scss index 7f173e291..d1103bd31 100644 --- a/src/tags/object/Paragraphs/Paragraphs.module.scss +++ b/src/tags/object/Paragraphs/Paragraphs.module.scss @@ -258,7 +258,7 @@ $border-thin: 1px solid rgba(137, 128, 152, 0.16); } } - .text { + .wrapperText { position: relative; .readingLine { diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index 8aced0b11..154a0da4d 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -175,12 +175,20 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } {v[item.namekey]} )} - - {v[item.textkey]} - {(isFF(FF_LSDV_E_278) && isActive) && ( - - )} - + {isFF(FF_LSDV_E_278) ? ( + + {(isFF(FF_LSDV_E_278) && isActive) && ( + + )} + + {v[item.textkey]} + + + ) : ( + + {v[item.textkey]} + + )} ); }); From 52103a4ac15613c0a3c0eba221efe74e3115ebe0 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Mon, 24 Jul 2023 17:14:30 -0300 Subject: [PATCH 13/20] removing the double FF validation --- src/tags/object/Paragraphs/Phrases.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index 154a0da4d..9d20c6bd1 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -177,7 +177,7 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } {isFF(FF_LSDV_E_278) ? ( - {(isFF(FF_LSDV_E_278) && isActive) && ( + {(isActive) && ( )} From c4c042e9a8beffd1cf6691918170529cb2bbc394 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Tue, 25 Jul 2023 12:23:43 -0300 Subject: [PATCH 14/20] trigger the auto scroll when user changes the author filter --- src/tags/object/Paragraphs/AuthorFilter.js | 4 +++- src/tags/object/Paragraphs/HtxParagraphs.js | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tags/object/Paragraphs/AuthorFilter.js b/src/tags/object/Paragraphs/AuthorFilter.js index e8aeaaf3d..f20f0558d 100644 --- a/src/tags/object/Paragraphs/AuthorFilter.js +++ b/src/tags/object/Paragraphs/AuthorFilter.js @@ -21,7 +21,7 @@ const renderMultipleSelected = (selected) => { ); }; -export const AuthorFilter = observer(({ item }) => { +export const AuthorFilter = observer(({ item, onChange }) => { const placeholder = useMemo(() => (Show all authors), []); const value = item.filterByAuthor; const options = useMemo(() => item._value.reduce((all, v) => all.includes(v[item.namekey]) ? all : [...all, v[item.namekey]], []).sort(), [item._value, item.namekey]); @@ -33,6 +33,8 @@ export const AuthorFilter = observer(({ item }) => { } else { item.setAuthorFilter(next); } + + onChange?.(); }, [item.setAuthorFilter]); return ( diff --git a/src/tags/object/Paragraphs/HtxParagraphs.js b/src/tags/object/Paragraphs/HtxParagraphs.js index bda15c409..c274f84af 100644 --- a/src/tags/object/Paragraphs/HtxParagraphs.js +++ b/src/tags/object/Paragraphs/HtxParagraphs.js @@ -513,7 +513,11 @@ class HtxParagraphsView extends Component { return (
{isFF(FF_DEV_2669) && ( - + { + setTimeout(() => { + this._handleScrollToPhrase(); + }, 100); // the transition phrase animation is 0.1s + }} /> )}
Date: Tue, 25 Jul 2023 12:36:32 -0300 Subject: [PATCH 15/20] change the timeout delay to get automatically from css --- src/tags/object/Paragraphs/HtxParagraphs.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/tags/object/Paragraphs/HtxParagraphs.js b/src/tags/object/Paragraphs/HtxParagraphs.js index c274f84af..23062a7bd 100644 --- a/src/tags/object/Paragraphs/HtxParagraphs.js +++ b/src/tags/object/Paragraphs/HtxParagraphs.js @@ -514,9 +514,12 @@ class HtxParagraphsView extends Component {
{isFF(FF_DEV_2669) && ( { + if (!this.activeRef.current) return; + const _timeoutDelay = parseFloat(window.getComputedStyle(this.activeRef.current).transitionDuration) * 1000; + setTimeout(() => { this._handleScrollToPhrase(); - }, 100); // the transition phrase animation is 0.1s + }, _timeoutDelay); }} /> )}
From 778d60c977c6a4e613f2f37edcb0b25a3b608096 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Tue, 25 Jul 2023 15:16:53 -0300 Subject: [PATCH 16/20] fix the overlap timing --- src/tags/object/Paragraphs/Phrases.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index 9d20c6bd1..b5a40715e 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -27,6 +27,7 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } const [isSeek, setIsSeek] = useState(null); const cls = item.layoutClasses; const withAudio = !!item.audio; + let observerTimeout; let observer; // default function to animate the reading line @@ -84,7 +85,14 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } } observer = new IntersectionObserver((entries) => { - setIsInViewport(entries[0].isIntersecting); + if(entries[0].isIntersecting) { + setIsInViewport(true); + clearTimeout(observerTimeout); + } else { + setTimeout(() => { + setIsInViewport(false); + }, 500); + } }, { rootMargin: '0px', }); From 16fe11ff63c9a886cc889d7ba3e49dfbe7e51765 Mon Sep 17 00:00:00 2001 From: Brandon Martel Date: Wed, 26 Jul 2023 10:09:39 -0500 Subject: [PATCH 17/20] fix playingId calc and reuse intersection observer --- src/tags/object/Paragraphs/Phrases.js | 41 +++++++++++++-------------- src/tags/object/Paragraphs/model.js | 2 +- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index b5a40715e..39adec37a 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -5,9 +5,9 @@ import { PauseCircleOutlined, PlayCircleOutlined } from '@ant-design/icons'; import styles from './Paragraphs.module.scss'; import { FF_LSDV_E_278, isFF } from '../../../utils/feature-flags'; import { IconPause, IconPlay } from '../../../assets/icons'; -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; -const formatTime = (seconds) => { +const formatTime = seconds => { if (isNaN(seconds)) return ''; const hours = Math.floor(seconds / 3600); @@ -25,10 +25,10 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } const [animationKeyFrame, setAnimationKeyFrame] = useState(null); const [seek, setSeek] = useState(0); const [isSeek, setIsSeek] = useState(null); + const prevReadlineRef = useRef(null); + const observerRef = useRef(null); const cls = item.layoutClasses; const withAudio = !!item.audio; - let observerTimeout; - let observer; // default function to animate the reading line const animateElement = useCallback( @@ -73,8 +73,16 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } // useRef to get the reading line element const readingLineRef = useCallback(node => { - if(observer) { - observer.disconnect(); + if (!observerRef.current) { + observerRef.current = new IntersectionObserver((entries) => { + setIsInViewport(entries[0].isIntersecting); + }, { + rootMargin: '0px', + }); + } + + if(observerRef.current && readingLineRef.current) { + observerRef.current.unobserve(readingLineRef.current); } if(node !== null) { @@ -84,20 +92,8 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } animateElement(node, 0, duration, item.playing); } - observer = new IntersectionObserver((entries) => { - if(entries[0].isIntersecting) { - setIsInViewport(true); - clearTimeout(observerTimeout); - } else { - setTimeout(() => { - setIsInViewport(false); - }, 500); - } - }, { - rootMargin: '0px', - }); - - observer.observe(node); + prevReadlineRef.current = node; + observerRef.current.observe(node); } }, [playingId]); @@ -111,7 +107,10 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } }); return () => { - observer?.disconnect(); + if (readingLineRef.current) { + observerRef.current?.unobserve(readingLineRef.current); + } + observerRef.current?.disconnect(); }; }, []); diff --git a/src/tags/object/Paragraphs/model.js b/src/tags/object/Paragraphs/model.js index fdbb9116c..e055f3f37 100644 --- a/src/tags/object/Paragraphs/model.js +++ b/src/tags/object/Paragraphs/model.js @@ -339,7 +339,7 @@ const PlayableAndSyncable = types.model() const regions = self.regionsValues; self.playingId = regions.findIndex(({ start, end }) => { - return currentTime >= start && currentTime <= end; + return currentTime >= start && currentTime < end; }); if (!audio.paused) { From dd30aac6fc821675a0d37e550c68b6ebf73e9360 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Wed, 26 Jul 2023 15:55:57 -0300 Subject: [PATCH 18/20] fix tests --- src/tags/object/Paragraphs/Phrases.js | 2 +- src/tags/object/Paragraphs/__tests__/Phrases.test.tsx | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index 39adec37a..863c6e38f 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -100,7 +100,7 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } useEffect(() => { if (!isFF(FF_LSDV_E_278)) return; - item.syncHandlers.set('seek', seek => { + item.syncHandlers?.set('seek', seek => { item.handleSyncPlay(seek); setSeek(seek); setIsInViewport(true); diff --git a/src/tags/object/Paragraphs/__tests__/Phrases.test.tsx b/src/tags/object/Paragraphs/__tests__/Phrases.test.tsx index e6c0a9e3c..571bc2b0d 100644 --- a/src/tags/object/Paragraphs/__tests__/Phrases.test.tsx +++ b/src/tags/object/Paragraphs/__tests__/Phrases.test.tsx @@ -8,6 +8,12 @@ import { FF_LSDV_E_278 } from '../../../../utils/feature-flags'; const ff = mockFF(); +const intersectionObserverMock = () => ({ + observe: () => null, +}); + +window.IntersectionObserver = jest.fn().mockImplementation(intersectionObserverMock); + jest.mock('mobx-state-tree', () => ({ ...jest.requireActual('mobx-state-tree'), getRoot: jest.fn(), From 995ea52c28b2c41491ef96a35f459248038fc57f Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Thu, 27 Jul 2023 11:01:44 -0300 Subject: [PATCH 19/20] revert observer change --- src/tags/object/Paragraphs/Phrases.js | 33 +++++++++++---------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index 863c6e38f..a55dd493e 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -5,7 +5,7 @@ import { PauseCircleOutlined, PlayCircleOutlined } from '@ant-design/icons'; import styles from './Paragraphs.module.scss'; import { FF_LSDV_E_278, isFF } from '../../../utils/feature-flags'; import { IconPause, IconPlay } from '../../../assets/icons'; -import { useCallback, useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useState } from 'react'; const formatTime = seconds => { if (isNaN(seconds)) return ''; @@ -25,10 +25,9 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } const [animationKeyFrame, setAnimationKeyFrame] = useState(null); const [seek, setSeek] = useState(0); const [isSeek, setIsSeek] = useState(null); - const prevReadlineRef = useRef(null); - const observerRef = useRef(null); const cls = item.layoutClasses; const withAudio = !!item.audio; + let observer; // default function to animate the reading line const animateElement = useCallback( @@ -73,16 +72,8 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } // useRef to get the reading line element const readingLineRef = useCallback(node => { - if (!observerRef.current) { - observerRef.current = new IntersectionObserver((entries) => { - setIsInViewport(entries[0].isIntersecting); - }, { - rootMargin: '0px', - }); - } - - if(observerRef.current && readingLineRef.current) { - observerRef.current.unobserve(readingLineRef.current); + if(observer) { + observer.disconnect(); } if(node !== null) { @@ -92,25 +83,27 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } animateElement(node, 0, duration, item.playing); } - prevReadlineRef.current = node; - observerRef.current.observe(node); + observer = new IntersectionObserver((entries) => { + setIsInViewport(entries[0].isIntersecting); + }, { + rootMargin: '0px', + }); + + observer.observe(node); } }, [playingId]); useEffect(() => { if (!isFF(FF_LSDV_E_278)) return; - item.syncHandlers?.set('seek', seek => { + item.syncHandlers.set('seek', seek => { item.handleSyncPlay(seek); setSeek(seek); setIsInViewport(true); }); return () => { - if (readingLineRef.current) { - observerRef.current?.unobserve(readingLineRef.current); - } - observerRef.current?.disconnect(); + observer?.disconnect(); }; }, []); From d9b4888040b0bc4e1d28eb3c6ffe803facafe619 Mon Sep 17 00:00:00 2001 From: Julio Sgarbi Date: Thu, 27 Jul 2023 14:26:18 -0300 Subject: [PATCH 20/20] fix tests --- src/tags/object/Paragraphs/Phrases.js | 2 +- src/tags/object/Paragraphs/__tests__/Phrases.test.tsx | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tags/object/Paragraphs/Phrases.js b/src/tags/object/Paragraphs/Phrases.js index a55dd493e..43d374ca3 100644 --- a/src/tags/object/Paragraphs/Phrases.js +++ b/src/tags/object/Paragraphs/Phrases.js @@ -96,7 +96,7 @@ export const Phrases = observer(({ item, playingId, activeRef, setIsInViewport } useEffect(() => { if (!isFF(FF_LSDV_E_278)) return; - item.syncHandlers.set('seek', seek => { + item.syncHandlers?.set('seek', seek => { item.handleSyncPlay(seek); setSeek(seek); setIsInViewport(true); diff --git a/src/tags/object/Paragraphs/__tests__/Phrases.test.tsx b/src/tags/object/Paragraphs/__tests__/Phrases.test.tsx index 571bc2b0d..1fc73e83e 100644 --- a/src/tags/object/Paragraphs/__tests__/Phrases.test.tsx +++ b/src/tags/object/Paragraphs/__tests__/Phrases.test.tsx @@ -10,6 +10,7 @@ const ff = mockFF(); const intersectionObserverMock = () => ({ observe: () => null, + disconnect: () => null, }); window.IntersectionObserver = jest.fn().mockImplementation(intersectionObserverMock);