From 02ddda587b7354f90c14e082be3170117606ef6c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 24 May 2020 14:11:25 +0100 Subject: [PATCH 1/5] Fix useEventEmitter to support passing all callback arguments Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/hooks/useEventEmitter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useEventEmitter.js b/src/hooks/useEventEmitter.js index 7adc6ef2e3..6a758fb108 100644 --- a/src/hooks/useEventEmitter.js +++ b/src/hooks/useEventEmitter.js @@ -32,7 +32,7 @@ export const useEventEmitter = (emitter, eventName, handler) => { if (!emitter) return; // Create event listener that calls handler function stored in ref - const eventListener = event => savedHandler.current(event); + const eventListener = (...args) => savedHandler.current(...args); // Add event listener emitter.on(eventName, eventListener); From cf7acded501e6fb34f165de0e6ae721f0a8edcfc Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 24 May 2020 14:12:16 +0100 Subject: [PATCH 2/5] Simply BaseAvatar hooks Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/avatars/BaseAvatar.js | 38 ++++++++-------------- 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/src/components/views/avatars/BaseAvatar.js b/src/components/views/avatars/BaseAvatar.js index 4fd614fbf7..d7a2aa14e5 100644 --- a/src/components/views/avatars/BaseAvatar.js +++ b/src/components/views/avatars/BaseAvatar.js @@ -30,54 +30,44 @@ const useImageUrl = ({url, urls, idName, name, defaultToInitialLetter}) => { const [imageUrls, setUrls] = useState([]); const [urlsIndex, setIndex] = useState(); - const onError = () => { - const nextIndex = urlsIndex + 1; - if (nextIndex < imageUrls.length) { - // try the next one - setIndex(nextIndex); - } - }; - - const defaultImageUrl = useMemo(() => AvatarLogic.defaultAvatarUrlForString(idName || name), [idName, name]); + const onError = useCallback(() => { + setIndex(i => i + 1); // try the next one + }, []); + const memoizedUrls = useMemo(() => urls, [JSON.stringify(urls)]); // eslint-disable-line react-hooks/exhaustive-deps useEffect(() => { // work out the full set of urls to try to load. This is formed like so: - // imageUrls: [ props.url, ...props.urls, default image ] + // imageUrls: [ props.url, ...props.urls ] let _urls = []; if (!SettingsStore.getValue("lowBandwidth")) { - _urls = urls || []; + _urls = memoizedUrls || []; if (url) { _urls.unshift(url); // put in urls[0] } } - if (defaultToInitialLetter) { - _urls.push(defaultImageUrl); // lowest priority - } - // deduplicate URLs _urls = Array.from(new Set(_urls)); setIndex(0); setUrls(_urls); - }, [url, ...(urls || [])]); // eslint-disable-line react-hooks/exhaustive-deps + }, [url, memoizedUrls]); // eslint-disable-line react-hooks/exhaustive-deps const cli = useContext(MatrixClientContext); const onClientSync = useCallback((syncState, prevState) => { // Consider the client reconnected if there is no error with syncing. // This means the state could be RECONNECTING, SYNCING, PREPARED or CATCHUP. const reconnected = syncState !== "ERROR" && prevState !== syncState; - if (reconnected && urlsIndex > 0 ) { // Did we fall back? - // Start from the highest priority URL again - setIndex(0); + if (reconnected) { + setIndex(i => i > 0 ? 0 : i); } - }, [urlsIndex]); + }, []); useEventEmitter(cli, "sync", onClientSync); const imageUrl = imageUrls[urlsIndex]; - return [imageUrl, imageUrl === defaultImageUrl, onError]; + return [imageUrl, onError]; }; const BaseAvatar = (props) => { @@ -96,9 +86,9 @@ const BaseAvatar = (props) => { ...otherProps } = props; - const [imageUrl, isDefault, onError] = useImageUrl({url, urls, idName, name, defaultToInitialLetter}); + const [imageUrl, onError] = useImageUrl({url, urls, idName, name, defaultToInitialLetter}); - if (isDefault) { + if (!imageUrl && defaultToInitialLetter) { const initialLetter = AvatarLogic.getInitialLetter(name); const textNode = ( { const imgNode = ( Date: Sun, 24 May 2020 14:25:31 +0100 Subject: [PATCH 3/5] fix tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- test/components/views/messages/TextualBody-test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/components/views/messages/TextualBody-test.js b/test/components/views/messages/TextualBody-test.js index 212afac5c4..07cd51edbd 100644 --- a/test/components/views/messages/TextualBody-test.js +++ b/test/components/views/messages/TextualBody-test.js @@ -205,9 +205,8 @@ describe("", () => { expect(content.html()).toBe('' + 'Hey ' + '' + - 'Member' + + 'Member' + ''); }); }); From 148f215d4eb9ce626b582992612092e1a643c7fd Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 25 May 2020 19:02:44 +0100 Subject: [PATCH 4/5] clean up Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/avatars/BaseAvatar.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/avatars/BaseAvatar.js b/src/components/views/avatars/BaseAvatar.js index d7a2aa14e5..4ece5baead 100644 --- a/src/components/views/avatars/BaseAvatar.js +++ b/src/components/views/avatars/BaseAvatar.js @@ -61,7 +61,7 @@ const useImageUrl = ({url, urls, idName, name, defaultToInitialLetter}) => { // This means the state could be RECONNECTING, SYNCING, PREPARED or CATCHUP. const reconnected = syncState !== "ERROR" && prevState !== syncState; if (reconnected) { - setIndex(i => i > 0 ? 0 : i); + setIndex(0); } }, []); useEventEmitter(cli, "sync", onClientSync); From 0861b1fbec91d639be97424ca618991855eeefb5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 25 May 2020 19:05:02 +0100 Subject: [PATCH 5/5] remove redundant props Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/avatars/BaseAvatar.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/avatars/BaseAvatar.js b/src/components/views/avatars/BaseAvatar.js index 4ece5baead..508691e5fd 100644 --- a/src/components/views/avatars/BaseAvatar.js +++ b/src/components/views/avatars/BaseAvatar.js @@ -26,7 +26,7 @@ import MatrixClientContext from "../../../contexts/MatrixClientContext"; import {useEventEmitter} from "../../../hooks/useEventEmitter"; import {toPx} from "../../../utils/units"; -const useImageUrl = ({url, urls, idName, name, defaultToInitialLetter}) => { +const useImageUrl = ({url, urls}) => { const [imageUrls, setUrls] = useState([]); const [urlsIndex, setIndex] = useState(); @@ -86,7 +86,7 @@ const BaseAvatar = (props) => { ...otherProps } = props; - const [imageUrl, onError] = useImageUrl({url, urls, idName, name, defaultToInitialLetter}); + const [imageUrl, onError] = useImageUrl({url, urls}); if (!imageUrl && defaultToInitialLetter) { const initialLetter = AvatarLogic.getInitialLetter(name);