From 2adafc53b75157909f1656addbe2fa1af882daad Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Thu, 31 Aug 2023 08:36:32 +0100 Subject: [PATCH 1/8] migrate index.js class to function component --- .../HTMLRenderers/PreRenderer/index.js | 82 +++++++++---------- 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js index efc9e432cba8..c1f33dfd56c0 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js @@ -1,29 +1,14 @@ -import React from 'react'; +import React, {useCallback, useEffect, useRef} from 'react'; import _ from 'underscore'; + +import ControlSelection from '../../../../libs/ControlSelection'; +import * as DeviceCapabilities from '../../../../libs/DeviceCapabilities'; import withLocalize from '../../../withLocalize'; import htmlRendererPropTypes from '../htmlRendererPropTypes'; import BasePreRenderer from './BasePreRenderer'; -import * as DeviceCapabilities from '../../../../libs/DeviceCapabilities'; -import ControlSelection from '../../../../libs/ControlSelection'; -class PreRenderer extends React.Component { - constructor(props) { - super(props); - - this.scrollNode = this.scrollNode.bind(this); - this.debouncedIsScrollingVertically = _.debounce(this.isScrollingVertically.bind(this), 100, true); - } - - componentDidMount() { - if (!this.ref) { - return; - } - this.ref.getScrollableNode().addEventListener('wheel', this.scrollNode); - } - - componentWillUnmount() { - this.ref.getScrollableNode().removeEventListener('wheel', this.scrollNode); - } +function PreRenderer(props) { + const scrollViewRef = useRef(); /** * Check if user is scrolling vertically based on deltaX and deltaY. We debounce this @@ -31,38 +16,51 @@ class PreRenderer extends React.Component { * @param {WheelEvent} event Wheel event * @returns {Boolean} true if user is scrolling vertically */ - isScrollingVertically(event) { + function isScrollingVertically(event) { // Mark as vertical scrolling only when absolute value of deltaY is more than the double of absolute // value of deltaX, so user can use trackpad scroll on the code block horizontally at a wide angle. return Math.abs(event.deltaY) > Math.abs(event.deltaX) * 2; } + const debouncedIsScrollingVertically = useCallback((event) => _.debounce(isScrollingVertically(event), 100, true), []); + /** * Manually scrolls the code block if code block horizontal scrollable, then prevents the event from being passed up to the parent. * @param {Object} event native event */ - scrollNode(event) { - const node = this.ref.getScrollableNode(); - const horizontalOverflow = node.scrollWidth > node.offsetWidth; - const isScrollingVertically = this.debouncedIsScrollingVertically(event); - if (event.currentTarget === node && horizontalOverflow && !isScrollingVertically) { - node.scrollLeft += event.deltaX; - event.preventDefault(); - event.stopPropagation(); + const scrollNode = useCallback( + (event) => { + const node = scrollViewRef.getScrollableNode(); + const horizontalOverflow = node.scrollWidth > node.offsetWidth; + if (event.currentTarget === node && horizontalOverflow && !debouncedIsScrollingVertically(event)) { + node.scrollLeft += event.deltaX; + event.preventDefault(); + event.stopPropagation(); + } + }, + [debouncedIsScrollingVertically], + ); + + useEffect(() => { + if (!scrollViewRef) { + return; } - } + scrollViewRef.getScrollableNode().addEventListener('wheel', scrollNode); - render() { - return ( - (this.ref = el)} - onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()} - onPressOut={() => ControlSelection.unblock()} - /> - ); - } + return () => { + scrollViewRef.getScrollableNode().removeEventListener('wheel', scrollNode); + }; + }, [scrollNode]); + + return ( + DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()} + onPressOut={() => ControlSelection.unblock()} + /> + ); } PreRenderer.propTypes = htmlRendererPropTypes; From 494cf78840449b3c59a1eccfbd5fb1227284190e Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Thu, 31 Aug 2023 12:27:44 +0100 Subject: [PATCH 2/8] use arrow function and useCallback --- .../HTMLRenderers/PreRenderer/index.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js index c1f33dfd56c0..8899e414440a 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js @@ -3,11 +3,10 @@ import _ from 'underscore'; import ControlSelection from '../../../../libs/ControlSelection'; import * as DeviceCapabilities from '../../../../libs/DeviceCapabilities'; -import withLocalize from '../../../withLocalize'; import htmlRendererPropTypes from '../htmlRendererPropTypes'; import BasePreRenderer from './BasePreRenderer'; -function PreRenderer(props) { +function PreRenderer({key, style, tnode, TDefaultRenderer}) { const scrollViewRef = useRef(); /** @@ -16,11 +15,11 @@ function PreRenderer(props) { * @param {WheelEvent} event Wheel event * @returns {Boolean} true if user is scrolling vertically */ - function isScrollingVertically(event) { + const isScrollingVertically = useCallback((event) => { // Mark as vertical scrolling only when absolute value of deltaY is more than the double of absolute // value of deltaX, so user can use trackpad scroll on the code block horizontally at a wide angle. return Math.abs(event.deltaY) > Math.abs(event.deltaX) * 2; - } + }, []); const debouncedIsScrollingVertically = useCallback((event) => _.debounce(isScrollingVertically(event), 100, true), []); @@ -54,8 +53,10 @@ function PreRenderer(props) { return ( DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()} onPressOut={() => ControlSelection.unblock()} @@ -64,5 +65,6 @@ function PreRenderer(props) { } PreRenderer.propTypes = htmlRendererPropTypes; +PreRenderer.displayName = 'PreRenderer'; -export default withLocalize(PreRenderer); +export default PreRenderer; From 4d748f1a0d3b34647ea6339b253624653625dd9c Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Fri, 1 Sep 2023 01:23:30 +0100 Subject: [PATCH 3/8] use current in ref and revert props destructure --- .../HTMLRenderers/PreRenderer/index.js | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js index 8899e414440a..d394fb474e51 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js @@ -6,22 +6,24 @@ import * as DeviceCapabilities from '../../../../libs/DeviceCapabilities'; import htmlRendererPropTypes from '../htmlRendererPropTypes'; import BasePreRenderer from './BasePreRenderer'; -function PreRenderer({key, style, tnode, TDefaultRenderer}) { +function PreRenderer(props) { const scrollViewRef = useRef(); /** - * Check if user is scrolling vertically based on deltaX and deltaY. We debounce this - * method in the constructor to make sure it's called only for the first event. + * Checks if user is scrolling vertically based on deltaX and deltaY. We debounce this + * method in order to make sure it's called only for the first event. * @param {WheelEvent} event Wheel event * @returns {Boolean} true if user is scrolling vertically */ - const isScrollingVertically = useCallback((event) => { - // Mark as vertical scrolling only when absolute value of deltaY is more than the double of absolute - // value of deltaX, so user can use trackpad scroll on the code block horizontally at a wide angle. - return Math.abs(event.deltaY) > Math.abs(event.deltaX) * 2; - }, []); + const isScrollingVertically = useCallback( + (event) => + // Mark as vertical scrolling only when absolute value of deltaY is more than the double of absolute + // value of deltaX, so user can use trackpad scroll on the code block horizontally at a wide angle. + Math.abs(event.deltaY) > Math.abs(event.deltaX) * 2, + [], + ); - const debouncedIsScrollingVertically = useCallback((event) => _.debounce(isScrollingVertically(event), 100, true), []); + const debouncedIsScrollingVertically = useCallback((event) => _.debounce(isScrollingVertically(event), 100, true), [isScrollingVertically]); /** * Manually scrolls the code block if code block horizontal scrollable, then prevents the event from being passed up to the parent. @@ -29,7 +31,7 @@ function PreRenderer({key, style, tnode, TDefaultRenderer}) { */ const scrollNode = useCallback( (event) => { - const node = scrollViewRef.getScrollableNode(); + const node = scrollViewRef.current.getScrollableNode(); const horizontalOverflow = node.scrollWidth > node.offsetWidth; if (event.currentTarget === node && horizontalOverflow && !debouncedIsScrollingVertically(event)) { node.scrollLeft += event.deltaX; @@ -41,25 +43,24 @@ function PreRenderer({key, style, tnode, TDefaultRenderer}) { ); useEffect(() => { - if (!scrollViewRef) { + if (!scrollViewRef.current) { return; } - scrollViewRef.getScrollableNode().addEventListener('wheel', scrollNode); + scrollViewRef.current.getScrollableNode().addEventListener('wheel', scrollNode); + const eventListenerRefValue = scrollViewRef.current; return () => { - scrollViewRef.getScrollableNode().removeEventListener('wheel', scrollNode); + eventListenerRefValue.getScrollableNode().removeEventListener('wheel', scrollNode); }; }, [scrollNode]); return ( DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()} - onPressOut={() => ControlSelection.unblock()} + onPressOut={ControlSelection.unblock()} /> ); } From 84f0b5292dfc6f3a28c6f5c09a955d7cabadee31 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Mon, 4 Sep 2023 09:51:56 +0100 Subject: [PATCH 4/8] Update src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js Co-authored-by: Michael (Mykhailo) Kravchenko --- .../HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js index d394fb474e51..3abf2a782146 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js @@ -60,7 +60,7 @@ function PreRenderer(props) { {...props} ref={scrollViewRef} onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()} - onPressOut={ControlSelection.unblock()} + onPressOut={ControlSelection.unblock} /> ); } From 568afb8972da1a21bc5428b24459ee29af3df345 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Tue, 26 Sep 2023 12:11:04 +0100 Subject: [PATCH 5/8] fix review comments --- .../HTMLRenderers/PreRenderer/index.js | 37 ++++++++----------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js index 3abf2a782146..7a0c51a069df 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js @@ -6,6 +6,13 @@ import * as DeviceCapabilities from '../../../../libs/DeviceCapabilities'; import htmlRendererPropTypes from '../htmlRendererPropTypes'; import BasePreRenderer from './BasePreRenderer'; +const isScrollingVertically = (event) => + // Mark as vertical scrolling only when absolute value of deltaY is more than the double of absolute + // value of deltaX, so user can use trackpad scroll on the code block horizontally at a wide angle. + Math.abs(event.deltaY) > Math.abs(event.deltaX) * 2; + +const debouncedIsScrollingVertically = (event) => _.debounce(isScrollingVertically(event), 100, true); + function PreRenderer(props) { const scrollViewRef = useRef(); @@ -15,32 +22,20 @@ function PreRenderer(props) { * @param {WheelEvent} event Wheel event * @returns {Boolean} true if user is scrolling vertically */ - const isScrollingVertically = useCallback( - (event) => - // Mark as vertical scrolling only when absolute value of deltaY is more than the double of absolute - // value of deltaX, so user can use trackpad scroll on the code block horizontally at a wide angle. - Math.abs(event.deltaY) > Math.abs(event.deltaX) * 2, - [], - ); - - const debouncedIsScrollingVertically = useCallback((event) => _.debounce(isScrollingVertically(event), 100, true), [isScrollingVertically]); /** * Manually scrolls the code block if code block horizontal scrollable, then prevents the event from being passed up to the parent. * @param {Object} event native event */ - const scrollNode = useCallback( - (event) => { - const node = scrollViewRef.current.getScrollableNode(); - const horizontalOverflow = node.scrollWidth > node.offsetWidth; - if (event.currentTarget === node && horizontalOverflow && !debouncedIsScrollingVertically(event)) { - node.scrollLeft += event.deltaX; - event.preventDefault(); - event.stopPropagation(); - } - }, - [debouncedIsScrollingVertically], - ); + const scrollNode = useCallback((event) => { + const node = scrollViewRef.current.getScrollableNode(); + const horizontalOverflow = node.scrollWidth > node.offsetWidth; + if (event.currentTarget === node && horizontalOverflow && !debouncedIsScrollingVertically(event)) { + node.scrollLeft += event.deltaX; + event.preventDefault(); + event.stopPropagation(); + } + }, []); useEffect(() => { if (!scrollViewRef.current) { From a577bc53b904d35a46ff40f09440e6292ad13bf9 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Wed, 27 Sep 2023 07:13:16 +0100 Subject: [PATCH 6/8] Update src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js Co-authored-by: Situ Chandra Shil <108292595+situchan@users.noreply.github.com> --- .../HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js index 7a0c51a069df..555a940e6f9b 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js @@ -38,11 +38,11 @@ function PreRenderer(props) { }, []); useEffect(() => { - if (!scrollViewRef.current) { + const eventListenerRefValue = scrollViewRef.current; + if (!eventListenerRefValue) { return; } - scrollViewRef.current.getScrollableNode().addEventListener('wheel', scrollNode); - const eventListenerRefValue = scrollViewRef.current; + eventListenerRefValue.getScrollableNode().addEventListener('wheel', scrollNode); return () => { eventListenerRefValue.getScrollableNode().removeEventListener('wheel', scrollNode); From ecda5f3a8f6809fdedce8115ba2c48f689f496f1 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Mon, 2 Oct 2023 04:48:32 +0100 Subject: [PATCH 7/8] Update src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js Co-authored-by: Situ Chandra Shil <108292595+situchan@users.noreply.github.com> --- .../HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js index 555a940e6f9b..5a39b1eeabbb 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js @@ -45,6 +45,9 @@ function PreRenderer(props) { eventListenerRefValue.getScrollableNode().addEventListener('wheel', scrollNode); return () => { + if (!eventListenerRefValue.getScrollableNode()) { + return; + } eventListenerRefValue.getScrollableNode().removeEventListener('wheel', scrollNode); }; }, [scrollNode]); From 9a2055bb1e50664f005aeca6f0430a6cfea7d469 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Mon, 2 Oct 2023 04:48:43 +0100 Subject: [PATCH 8/8] Update src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js Co-authored-by: Situ Chandra Shil <108292595+situchan@users.noreply.github.com> --- .../HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js index 5a39b1eeabbb..782ad82f643c 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer/index.js @@ -11,7 +11,7 @@ const isScrollingVertically = (event) => // value of deltaX, so user can use trackpad scroll on the code block horizontally at a wide angle. Math.abs(event.deltaY) > Math.abs(event.deltaX) * 2; -const debouncedIsScrollingVertically = (event) => _.debounce(isScrollingVertically(event), 100, true); +const debouncedIsScrollingVertically = _.debounce(isScrollingVertically, 100, true); function PreRenderer(props) { const scrollViewRef = useRef();