From f7a08ed2a0f008ee534b863fd1fe7cdf1771355a Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Wed, 6 Nov 2019 17:26:15 +1100 Subject: [PATCH 1/3] fixing issue with announcer and hot module reloading --- docs/support/media.md | 1 + src/view/use-announcer/use-announcer.js | 56 +++++++++++++------------ 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/docs/support/media.md b/docs/support/media.md index b2765adc7c..0643bfd9d9 100644 --- a/docs/support/media.md +++ b/docs/support/media.md @@ -30,6 +30,7 @@ This page contains a list of articles, blogs and newsletters that `react-beautif ## Newsletters +- Tiny letter [November 04, 2019](https://tinyletter.com/cassidoo/letters/the-world-is-changed-by-your-example-not-by-your-opinion-paulo-coelho) - Ponyfoo [issue 78](https://ponyfoo.com/weekly/78/javascript-and-css-engines-pwa-drag-and-drop-web-components-and-http-2) - PonyFoo [issue 99](https://ponyfoo.com/weekly/99/react-across-the-universe-typography-load-balancing-and-javascript-frameworks) - PonyFoo [issue 141](https://ponyfoo.com/weekly/141/http-3-bgp-leaks-react-as-native-dom-typescript-tensorflow-and-all-things-performance) diff --git a/src/view/use-announcer/use-announcer.js b/src/view/use-announcer/use-announcer.js index 1c7985faef..2f52b03904 100644 --- a/src/view/use-announcer/use-announcer.js +++ b/src/view/use-announcer/use-announcer.js @@ -14,42 +14,46 @@ export default function useAnnouncer(contextId: ContextId): Announce { const id: string = useMemo(() => getId(contextId), [contextId]); const ref = useRef(null); - useEffect(() => { - invariant(!ref.current, 'Announcement node already mounted'); + useEffect( + function setup() { + invariant(!ref.current, 'Announcement node already mounted'); - const el: HTMLElement = document.createElement('div'); - ref.current = el; + const el: HTMLElement = document.createElement('div'); + ref.current = el; - // identifier - el.id = id; + // identifier + el.id = id; - // Aria live region + // Aria live region - // will force itself to be read - el.setAttribute('aria-live', 'assertive'); - el.setAttribute('role', 'log'); - // must read the whole thing every time - el.setAttribute('aria-atomic', 'true'); + // will force itself to be read + el.setAttribute('aria-live', 'assertive'); + el.setAttribute('role', 'log'); + // must read the whole thing every time + el.setAttribute('aria-atomic', 'true'); - // hide the element visually - Object.assign(el.style, visuallyHidden); + // hide the element visually + Object.assign(el.style, visuallyHidden); - // Add to body - getBodyElement().appendChild(el); + // Add to body + getBodyElement().appendChild(el); - return () => { - // unmounting after a timeout to let any annoucements - // during a mount be published - setTimeout(function remove() { + return function cleanup() { + // grabbing and clearing ref incase effect is about to run again const toBeRemoved: ?HTMLElement = ref.current; invariant(toBeRemoved, 'Cannot unmount announcement node'); - - // Remove from body - getBodyElement().removeChild(toBeRemoved); ref.current = null; - }); - }; - }, [id]); + + // unmounting after a timeout to let any annoucements + // during a mount be published + setTimeout(function remove() { + // Remove from body + getBodyElement().removeChild(toBeRemoved); + }); + }; + }, + [id], + ); const announce: Announce = useCallback((message: string): void => { const el: ?HTMLElement = ref.current; From 6e9e8ecc033011e9b9dee4245e0325bf28075614 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Wed, 6 Nov 2019 20:20:44 +1100 Subject: [PATCH 2/3] fixing broken tests --- src/view/use-announcer/use-announcer.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/view/use-announcer/use-announcer.js b/src/view/use-announcer/use-announcer.js index 2f52b03904..20c33050d1 100644 --- a/src/view/use-announcer/use-announcer.js +++ b/src/view/use-announcer/use-announcer.js @@ -1,7 +1,6 @@ // @flow import { useRef, useEffect } from 'react'; import { useMemo, useCallback } from 'use-memo-one'; -import { invariant } from '../../invariant'; import type { Announce, ContextId } from '../../types'; import { warning } from '../../dev-warning'; import getBodyElement from '../get-body-element'; @@ -16,9 +15,8 @@ export default function useAnnouncer(contextId: ContextId): Announce { useEffect( function setup() { - invariant(!ref.current, 'Announcement node already mounted'); - const el: HTMLElement = document.createElement('div'); + // storing reference for usage in announce ref.current = el; // identifier @@ -39,16 +37,13 @@ export default function useAnnouncer(contextId: ContextId): Announce { getBodyElement().appendChild(el); return function cleanup() { - // grabbing and clearing ref incase effect is about to run again - const toBeRemoved: ?HTMLElement = ref.current; - invariant(toBeRemoved, 'Cannot unmount announcement node'); - ref.current = null; + // Not clearing the ref as it might be used by announce before the timeout expires - // unmounting after a timeout to let any annoucements + // unmounting after a timeout to let any announcements // during a mount be published setTimeout(function remove() { - // Remove from body - getBodyElement().removeChild(toBeRemoved); + // not clearing the ref as it might have been set by a new effect + getBodyElement().removeChild(el); }); }; }, From 669d94f39e8e46574aaafc336778e352d0916078 Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Wed, 6 Nov 2019 20:24:29 +1100 Subject: [PATCH 3/3] adding logic for ref clearing --- src/view/use-announcer/use-announcer.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/view/use-announcer/use-announcer.js b/src/view/use-announcer/use-announcer.js index 20c33050d1..ed571c8269 100644 --- a/src/view/use-announcer/use-announcer.js +++ b/src/view/use-announcer/use-announcer.js @@ -44,6 +44,12 @@ export default function useAnnouncer(contextId: ContextId): Announce { setTimeout(function remove() { // not clearing the ref as it might have been set by a new effect getBodyElement().removeChild(el); + + // if el was the current ref - clear it so that + // we can get a warning if announce is called + if (el === ref.current) { + ref.current = null; + } }); }; },