From 482e52ec1f93f06e418fe907bd177da10daecaff Mon Sep 17 00:00:00 2001 From: John Davey <66066685+johndavey72@users.noreply.github.com> Date: Mon, 22 Nov 2021 17:25:55 +0000 Subject: [PATCH] GN-117 Remove need to assert on dom (#148) * context experiments * basic context setup * tidy up debugging * uncommen event listener for expected closing behaviour * make debug clearer * clean up console logging for clarity * move globalnavcontext provider up to header component * GN-117 add useCallback to clickOutside hook to maintain reference to id of open dropdown * GN-117 Refactor Nav component to class. Add custom hook and context for open dropdown state and nav area click outside detection * GN-117 Begin conversion of Account class component to functional component * GN-117 Remove commented out lines * GN-117 Reinstate propTypes for Naccount placeholder component * Side effect sideline (#150) * side effect shenanigans * Magical useRef() * update key event to use e.key rather than e.keycode for better cross platform * Move myAccount isExpanded state into GlobalNavContext * Remove redundant event binding * Make megamenu click detection more 'reacty' * Manage context of opened menus * Rename Naccount component to Account Co-authored-by: John Davey * Tests needed * GN-117 Tidy up redundant arguments and commented out logic for useClickOutside hook * GN-117 Change directory structure for hooks and react context. Make useClickOutside re-useable. * GN-117 Import GlobalNavContextProvider from new location for Account tests * GN-117 Fixing broken unit tests * GN-117 Fixing broken tests * GN-117 Fixing broken tests * Replace simulate click * GN-117 Revert some tests to shallow rendering after adding defaults to context * GN-117 Revert to shallow mounting for a previously failing unit test * GN-117 Convert unit test from shallow to mount due to issues around enzyme support of useContext 'https://github.com/enzymejs/enzyme/issues/2176' * GN-117 Rename GlobalNavContext to HeaderContext * GN-117 remove unused dependencies from account test * GN-117 Tidy up unit tests * testing custom hook useClickOutside * basic setup * Custom hook test * Update test * Rename test Co-authored-by: w@rren --- src/Header/Account/Account.jsx | 245 ++++++++--------- src/Header/Account/Account.test.jsx | 52 ++-- src/Header/Account/AccountOld.jsx | 251 ++++++++++++++++++ .../__snapshots__/Account.test.jsx.snap | 4 +- src/Header/Header.jsx | 5 +- src/Header/Nav/Nav.jsx | 232 ++++++++-------- src/Header/Nav/NavLinks/NavLinks.jsx | 33 +-- src/Header/__snapshots__/Header.test.jsx.snap | 4 +- src/Header/context/HeaderContext.js | 36 +++ src/hooks/useClickOutside.js | 25 ++ src/hooks/useClickOutside.test.jsx | 36 +++ 11 files changed, 614 insertions(+), 309 deletions(-) create mode 100644 src/Header/Account/AccountOld.jsx create mode 100644 src/Header/context/HeaderContext.js create mode 100644 src/hooks/useClickOutside.js create mode 100644 src/hooks/useClickOutside.test.jsx diff --git a/src/Header/Account/Account.jsx b/src/Header/Account/Account.jsx index af13479e..aa3f41e1 100644 --- a/src/Header/Account/Account.jsx +++ b/src/Header/Account/Account.jsx @@ -1,4 +1,11 @@ -import React, { Component } from "react"; +import React, { + useContext, + useEffect, + useState, + useCallback, + useRef, +} from "react"; +import { HeaderContext } from "../context/HeaderContext"; import PropTypes from "prop-types"; import classnames from "classnames"; @@ -13,63 +20,42 @@ import { headerClickEventAction, } from "../../tracker"; -const escapeKeyCode = 27; +function Account(props) { + // We've left this is state as per the pre-hook implementation + const [doesUseIdAM] = useState(props.provider == Account.providers.idam); -export default class Account extends Component { - constructor(props) { - super(props); + const { accountMenuIsExpanded, setAccountMenuIsExpanded } = + useContext(HeaderContext); - this.state = { - isExpanded: false, - useIdAM: this.props.provider == Account.providers.idam, - }; + const keypress = useRef(), + myAccountButton = useRef(), + myAccountMenu = useRef(); - this.handleMyAccountButtonClick = - this.handleMyAccountButtonClick.bind(this); - this.handleKeyDown = this.handleKeyDown.bind(this); - this.handleMenuItemClick = this.handleMenuItemClick.bind(this); - this.handleMegaMenuClick = this.handleMegaMenuClick.bind(this); - } + const handleMyAccountButtonClick = useCallback((e) => { + keypress.current = e.pageX; + setAccountMenuIsExpanded((prevState) => !prevState); + }, []); - handleMyAccountButtonClick(e) { - const isKeyboardEvent = !e.pageX; - this.setState( - function (prevState) { - return { isExpanded: !prevState.isExpanded }; - }, - function () { - if (this.state.isExpanded && isKeyboardEvent) { - const accountMenu = document.getElementById("my-account"); - accountMenu.setAttribute("tabIndex", -1); - accountMenu.focus(); - } - }.bind(this) - ); + function focusAccount() { + myAccountMenu && myAccountMenu.current.setAttribute("tabIndex", -1); + myAccountMenu && myAccountMenu.current.focus(); } - handleKeyDown(event) { - if (event.keyCode === escapeKeyCode) { - event.preventDefault(); - this.setState({ - isExpanded: false, - }); - document.getElementById("my-account-button").focus(); + useEffect(() => { + if (!keypress.current && accountMenuIsExpanded) { + focusAccount(); } - } + }, [accountMenuIsExpanded]); - // NOTE: We would benefit from managing the state higher up - handleMegaMenuClick(event) { - let megaMenu = document.querySelector("#header-menu"); - - if (megaMenu.contains(event.target)) { - this.setState({ - isExpanded: false, - }); - megaMenu.focus(); + function handleKeyDown(e) { + if (e.key === "Escape") { + e.preventDefault(); + setAccountMenuIsExpanded(false); + myAccountButton && myAccountButton.current.focus(); } } - handleMenuItemClick(e) { + function handleMenuItemClick(e) { const href = e.currentTarget.getAttribute("href"); let eventLabel; @@ -98,20 +84,20 @@ export default class Account extends Component { } } - componentDidMount() { + useEffect(() => { const consultationsResponsesLink = { "Consultation responses": "https://www.nice.org.uk/consultations/", }; - if (this.state.useIdAM) { + if (doesUseIdAM) { //nice accounts supplies links like: {"John Holland":"https://accounts.nice.org.uk/users/143980/editprofile","Sign out":"https://accounts.nice.org.uk/signout"} //idam supplies links like:[{ key: "My profile", value: "/Account/todo" },{ key: "Sign out", value: "/Account/Logout" }] //the following just converts the idam format to the nice accounts format. - const { displayName } = this.props; + const { displayName } = props; const isLoggedIn = !!displayName; - let links = this.props.links.reduce(function (links, link) { + let links = props.links.reduce(function (links, link) { links[link.text] = link.url; return links; }, {}); @@ -125,99 +111,90 @@ export default class Account extends Component { links: links, }; - if (this.props.onLoginStatusChecked) { - this.props.onLoginStatusChecked(convertedData); + if (props.onLoginStatusChecked) { + props.onLoginStatusChecked(convertedData); } } else { //NICE accounts - niceAccountsLoggedIn(this.props.environment) - .then( - function (data) { - if (this.props.onLoginStatusChecked) { - data.links = { ...consultationsResponsesLink, ...data.links }; - - this.props.onLoginStatusChecked(data); - } - }.bind(this) - ) - .catch( - function (e) { - console.warn("Couldn't load account data from NICE accounts", e); - }.bind(this) - ); + niceAccountsLoggedIn(props.environment) + .then(function (data) { + if (props.onLoginStatusChecked) { + data.links = { ...consultationsResponsesLink, ...data.links }; + props.onLoginStatusChecked(data); + } + }) + .catch(function (e) { + console.warn("Couldn't load account data from NICE accounts", e); + }); } + }, []); - document.addEventListener("click", this.handleMegaMenuClick); - } - - render() { - const { accountsData, environment } = this.props; + const { accountsData, environment } = props; - let signInLink = {}; - if (this.state.useIdAM) { - signInLink = this.props.links[0]; - } else { - signInLink["text"] = "Sign in"; - signInLink["url"] = getDomainBaseUrl(environment) + "signin"; - } + let signInLink = {}; + if (doesUseIdAM) { + signInLink = props.links[0]; + } else { + signInLink["text"] = "Sign in"; + signInLink["url"] = getDomainBaseUrl(environment) + "signin"; + } - return this.props.isLoggedIn ? ( -
- - -
- ) : ( - + + + + ) : ( + + {signInLink.text} + + ); } +export default Account; + Account.providers = { idam: "idam", niceAccounts: "niceAccounts", diff --git a/src/Header/Account/Account.test.jsx b/src/Header/Account/Account.test.jsx index 14e7c08d..ba2bc087 100644 --- a/src/Header/Account/Account.test.jsx +++ b/src/Header/Account/Account.test.jsx @@ -2,6 +2,7 @@ import React from "react"; import Account from "./Account"; import { shallow, mount } from "enzyme"; import toJson from "enzyme-to-json"; +import { HeaderContextProvider } from "./../context/HeaderContext"; import { eventName, @@ -10,8 +11,6 @@ import { eventTimeout, } from "../../tracker"; -const escapeKeyCode = 27; - jest.mock("./nice-accounts", () => ({ checkIsLoggedIn: jest.fn(() => Promise.resolve({ @@ -49,7 +48,6 @@ describe("Account", () => { it("Matches snapshot when logged out", () => { const wrapper = shallow(); - expect(toJson(wrapper)).toMatchSnapshot(); }); @@ -63,20 +61,19 @@ describe("Account", () => { it("Matches snapshot when logged out using IDAM", () => { const wrapper = shallow(); - expect(toJson(wrapper)).toMatchSnapshot(); }); - it("Matches snapshot when logged in using IDAM", () => { + it("Matches snapshot when logged in using IDAM", () => { const wrapper = shallow( ); - expect(toJson(wrapper)).toMatchSnapshot(); }); it("Is the correct authentication environment when supplied", () => { const wrapper = shallow(); + expect(wrapper.find(".button").props().href).toBe( "https://beta-accounts.nice.org.uk/signin" ); @@ -85,8 +82,13 @@ describe("Account", () => { it("Calls onLoginStatusChecked callback prop when mounted", (done) => { const onLoginStatusChecked = jest.fn(); - shallow( - + mount( + + + ); setImmediate(() => { @@ -106,13 +108,15 @@ describe("Account", () => { document.body.appendChild(appContainer); const wrapper = mount( - , + + + , { attachTo: appContainer } ); wrapper.find("#my-account-button").simulate("click", { pageX: 99 }); - - wrapper.instance().forceUpdate(); + // wrapper.instance().forceUpdate(); + wrapper.update(); expect(wrapper.find("#my-account-button").props()["aria-expanded"]).toBe( true @@ -127,7 +131,9 @@ describe("Account", () => { document.body.appendChild(appContainer); const wrapper = mount( - , + + + , { attachTo: appContainer } ); @@ -144,17 +150,15 @@ describe("Account", () => { document.body.appendChild(appContainer); const wrapper = mount( - , + + + , { attachTo: appContainer } ); wrapper.find("#my-account-button").simulate("click", {}); - wrapper - .find("#my-account-button") - .simulate("keydown", { keyCode: escapeKeyCode }); - - wrapper.instance().forceUpdate(); - + wrapper.find("#my-account-button").simulate("keydown", { key: "Escape" }); + wrapper.update(); expect(wrapper.find("#my-account-button").props()["aria-expanded"]).toBe( false ); @@ -166,7 +170,9 @@ describe("Account", () => { document.body.appendChild(appContainer); const wrapper = mount( - , + + + , { attachTo: appContainer } ); @@ -175,7 +181,7 @@ describe("Account", () => { wrapper .find("a[role='menuitem']") .first() - .simulate("keydown", { keyCode: escapeKeyCode }); + .simulate("keydown", { key: "Escape" }); expect( wrapper.find("[aria-controls='my-account']").props()["aria-expanded"] @@ -203,7 +209,9 @@ describe("Account", () => { const preventDefault = jest.fn(); - wrapper.find(`a[children="${linkText}"]`).simulate("click", { + const button = wrapper.find(`a[children="${linkText}"]`); + + button.props().onClick({ preventDefault: preventDefault, currentTarget: { getAttribute: () => href, diff --git a/src/Header/Account/AccountOld.jsx b/src/Header/Account/AccountOld.jsx new file mode 100644 index 00000000..af13479e --- /dev/null +++ b/src/Header/Account/AccountOld.jsx @@ -0,0 +1,251 @@ +import React, { Component } from "react"; +import PropTypes from "prop-types"; +import classnames from "classnames"; + +import { + checkIsLoggedIn as niceAccountsLoggedIn, + getDomainBaseUrl, +} from "./nice-accounts"; +import styles from "./Account.module.scss"; +import { + trackEvent, + defaultEventCategory, + headerClickEventAction, +} from "../../tracker"; + +const escapeKeyCode = 27; + +export default class Account extends Component { + constructor(props) { + super(props); + + this.state = { + isExpanded: false, + useIdAM: this.props.provider == Account.providers.idam, + }; + + this.handleMyAccountButtonClick = + this.handleMyAccountButtonClick.bind(this); + this.handleKeyDown = this.handleKeyDown.bind(this); + this.handleMenuItemClick = this.handleMenuItemClick.bind(this); + this.handleMegaMenuClick = this.handleMegaMenuClick.bind(this); + } + + handleMyAccountButtonClick(e) { + const isKeyboardEvent = !e.pageX; + this.setState( + function (prevState) { + return { isExpanded: !prevState.isExpanded }; + }, + function () { + if (this.state.isExpanded && isKeyboardEvent) { + const accountMenu = document.getElementById("my-account"); + accountMenu.setAttribute("tabIndex", -1); + accountMenu.focus(); + } + }.bind(this) + ); + } + + handleKeyDown(event) { + if (event.keyCode === escapeKeyCode) { + event.preventDefault(); + this.setState({ + isExpanded: false, + }); + document.getElementById("my-account-button").focus(); + } + } + + // NOTE: We would benefit from managing the state higher up + handleMegaMenuClick(event) { + let megaMenu = document.querySelector("#header-menu"); + + if (megaMenu.contains(event.target)) { + this.setState({ + isExpanded: false, + }); + megaMenu.focus(); + } + } + + handleMenuItemClick(e) { + const href = e.currentTarget.getAttribute("href"); + + let eventLabel; + + if (href.indexOf("editprofile") > -1) { + eventLabel = "Edit profile"; + } else if (href.indexOf("signout") > -1) { + eventLabel = "Sign out"; + } else if (href.indexOf("signin") > -1) { + eventLabel = "Sign in"; + } + + if (eventLabel) { + e.preventDefault(); + + trackEvent( + defaultEventCategory, + headerClickEventAction, + eventLabel, + null, + href, + function () { + window.location.href = href; + } + ); + } + } + + componentDidMount() { + const consultationsResponsesLink = { + "Consultation responses": "https://www.nice.org.uk/consultations/", + }; + + if (this.state.useIdAM) { + //nice accounts supplies links like: {"John Holland":"https://accounts.nice.org.uk/users/143980/editprofile","Sign out":"https://accounts.nice.org.uk/signout"} + //idam supplies links like:[{ key: "My profile", value: "/Account/todo" },{ key: "Sign out", value: "/Account/Logout" }] + //the following just converts the idam format to the nice accounts format. + + const { displayName } = this.props; + const isLoggedIn = !!displayName; + + let links = this.props.links.reduce(function (links, link) { + links[link.text] = link.url; + return links; + }, {}); + + if (isLoggedIn) { + links = { ...consultationsResponsesLink, ...links }; + } + + const convertedData = { + display_name: displayName, + links: links, + }; + + if (this.props.onLoginStatusChecked) { + this.props.onLoginStatusChecked(convertedData); + } + } else { + //NICE accounts + niceAccountsLoggedIn(this.props.environment) + .then( + function (data) { + if (this.props.onLoginStatusChecked) { + data.links = { ...consultationsResponsesLink, ...data.links }; + + this.props.onLoginStatusChecked(data); + } + }.bind(this) + ) + .catch( + function (e) { + console.warn("Couldn't load account data from NICE accounts", e); + }.bind(this) + ); + } + + document.addEventListener("click", this.handleMegaMenuClick); + } + + render() { + const { accountsData, environment } = this.props; + + let signInLink = {}; + if (this.state.useIdAM) { + signInLink = this.props.links[0]; + } else { + signInLink["text"] = "Sign in"; + signInLink["url"] = getDomainBaseUrl(environment) + "signin"; + } + + return this.props.isLoggedIn ? ( +
+ + +
+ ) : ( + + {signInLink.text} + + ); + } +} + +Account.providers = { + idam: "idam", + niceAccounts: "niceAccounts", +}; + +Account.propTypes = { + isLoggedIn: PropTypes.bool.isRequired, + onLoginStatusChecked: PropTypes.func, + accountsData: PropTypes.shape({ + display_name: PropTypes.string, + thumbnail: PropTypes.string, + links: PropTypes.object, + }), + environment: PropTypes.oneOf(["live", "test", "beta", "local"]), + provider: PropTypes.oneOf([ + Account.providers.niceAccounts, + Account.providers.idam, + ]), + links: PropTypes.arrayOf( + PropTypes.shape({ + text: PropTypes.string.isRequired, + url: PropTypes.string.isRequired, + }) + ), + displayName: PropTypes.string, +}; + +Account.defaultProps = { + environment: "live", + provider: "niceAccounts", +}; diff --git a/src/Header/Account/__snapshots__/Account.test.jsx.snap b/src/Header/Account/__snapshots__/Account.test.jsx.snap index 85884244..691d5ef7 100644 --- a/src/Header/Account/__snapshots__/Account.test.jsx.snap +++ b/src/Header/Account/__snapshots__/Account.test.jsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Account Matches snapshot when logged in using IDAM 1`] = ` +exports[`Account Matches snapshot when logged in 1`] = `
@@ -55,7 +55,7 @@ exports[`Account Matches snapshot when logged in using IDAM 1`] = `
`; -exports[`Account Matches snapshot when logged in 1`] = ` +exports[`Account Matches snapshot when logged in using IDAM 1`] = `
diff --git a/src/Header/Header.jsx b/src/Header/Header.jsx index 54c73d64..e878ecdd 100644 --- a/src/Header/Header.jsx +++ b/src/Header/Header.jsx @@ -15,6 +15,7 @@ import Account from "./Account"; import SkipLink from "./SkipLink"; import styles from "./Header.module.scss"; +import { HeaderContextProvider } from "./context/HeaderContext"; export class Header extends Component { constructor(props) { @@ -84,7 +85,7 @@ export class Header extends Component { render() { return ( this.props.enabled !== false && ( - <> +
- + ) ); } diff --git a/src/Header/Nav/Nav.jsx b/src/Header/Nav/Nav.jsx index 5249a30a..0c7a1139 100644 --- a/src/Header/Nav/Nav.jsx +++ b/src/Header/Nav/Nav.jsx @@ -1,4 +1,4 @@ -import React, { Component } from "react"; +import React, { useContext } from "react"; import PropTypes from "prop-types"; import classnames from "classnames"; @@ -11,34 +11,19 @@ import { headerClickEventAction, } from "../../tracker"; -export default class Nav extends Component { - constructor(props) { - super(props); +import useClickOutside from "../../hooks/useClickOutside"; +import { HeaderContext } from "../context/HeaderContext"; - this.handleAccountNavItemClick = this.handleAccountNavItemClick.bind(this); - } - - handleNavItemClick(e) { - e.preventDefault(); - - const href = e.currentTarget.getAttribute("href"); - - // To support IE8 - const eventLabel = e.currentTarget.textContent || e.currentTarget.innerText; +function Nav(props) { + const { accountsLinks } = props; + const context = useContext(HeaderContext); + const { ref } = useClickOutside(wasClickedOutside); - trackEvent( - defaultEventCategory, - headerClickEventAction, - eventLabel, - null, - href, - function () { - window.location.href = href; - } - ); + function wasClickedOutside(result) { + if (result) context.setidOfOpenDropdown(null); } - handleAccountNavItemClick(e) { + function handleAccountNavItemClick(e) { const href = e.currentTarget.getAttribute("href"); let eventLabel; @@ -65,114 +50,113 @@ export default class Nav extends Component { } } - render() { - const { accountsLinks } = this.props; - - // Links from NICE Accounts is an object so flatten to a loop for easier traversal - const accountsLinksArray = - accountsLinks && - Object.keys(accountsLinks).map((text) => ({ - text: text, - href: accountsLinks[text], - })); - - // Would need to polyfill Array.prototype.find to rewrite these loops, whilst we support IE - // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find for support - let activeService = null; - let internalService = false; - let servicesToDisplay = services.external; //default to displaying external services. - for (let i = 0; i < services.internal.length; i++) { - const internalRootLink = services.internal[i]; - if (this.props.service && internalRootLink.id === this.props.service) { - internalService = true; - activeService = internalRootLink; - servicesToDisplay = [internalRootLink]; //unlike external, internal services dosn't display other internal services. - break; - } + // Links from NICE Accounts is an object so flatten to a loop for easier traversal + const accountsLinksArray = + accountsLinks && + Object.keys(accountsLinks).map((text) => ({ + text: text, + href: accountsLinks[text], + })); + + // Would need to polyfill Array.prototype.find to rewrite these loops, whilst we support IE + // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find for support + let activeService = null; + let internalService = false; + let servicesToDisplay = services.external; //default to displaying external services. + for (let i = 0; i < services.internal.length; i++) { + const internalRootLink = services.internal[i]; + if (props.service && internalRootLink.id === props.service) { + internalService = true; + activeService = internalRootLink; + servicesToDisplay = [internalRootLink]; //unlike external, internal services dosn't display other internal services. + break; } - if (!internalService) { - for (let i = 0; i < services.external.length; i++) { - const externalService = services.external[i]; - if (this.props.service && externalService.id === this.props.service) { - activeService = externalService; - break; - } + } + if (!internalService) { + for (let i = 0; i < services.external.length; i++) { + const externalService = services.external[i]; + if (props.service && externalService.id === props.service) { + activeService = externalService; + break; } } - let additionalSubMenuLinks = []; - for (let i = 0; i < this.props.additionalSubMenuItems.length; i++) { - const additionalSubMenuItem = this.props.additionalSubMenuItems[i]; - if ( - typeof additionalSubMenuItem !== "undefined" && - additionalSubMenuItem.service === this.props.service && - Array.isArray(additionalSubMenuItem.links) - ) { - additionalSubMenuLinks = additionalSubMenuItem.links.map((link) => ({ - text: link.text, - href: link.url, - })); - } + } + let additionalSubMenuLinks = []; + for (let i = 0; i < props.additionalSubMenuItems.length; i++) { + const additionalSubMenuItem = props.additionalSubMenuItems[i]; + if ( + typeof additionalSubMenuItem !== "undefined" && + additionalSubMenuItem.service === props.service && + Array.isArray(additionalSubMenuItem.links) + ) { + additionalSubMenuLinks = additionalSubMenuItem.links.map((link) => ({ + text: link.text, + href: link.url, + })); } - const subLinks = - activeService && - activeService.links && - activeService.links.concat(additionalSubMenuLinks); - - return ( -
-
+ ); } +export default Nav; + Nav.propTypes = { skipLinkId: PropTypes.string, service: PropTypes.string, diff --git a/src/Header/Nav/NavLinks/NavLinks.jsx b/src/Header/Nav/NavLinks/NavLinks.jsx index 248f3d3f..e9a37bcc 100644 --- a/src/Header/Nav/NavLinks/NavLinks.jsx +++ b/src/Header/Nav/NavLinks/NavLinks.jsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from "react"; +import React, { useState, useEffect, useContext } from "react"; import classnames from "classnames"; import FocusTrap from "focus-trap-react"; import PropTypes from "prop-types"; @@ -7,6 +7,7 @@ import ChevronDown from "@nice-digital/icons/lib/ChevronDown"; import ChevronUp from "@nice-digital/icons/lib/ChevronUp"; import SubNav from "../SubNav"; import Dropdown from "../Dropdown"; +import { HeaderContext } from "../../context/HeaderContext"; import styles from "./NavLinks.module.scss"; import { @@ -23,8 +24,8 @@ export function NavLinks({ skipLinkId, handleScrim, }) { - const [idOfOpenDropdown, setidOfOpenDropdown] = useState(null); const [canUseDOM, setCanUseDOM] = useState(false); + const { idOfOpenDropdown, setidOfOpenDropdown } = useContext(HeaderContext); useEffect(() => { setCanUseDOM(true); @@ -61,22 +62,6 @@ export function NavLinks({ if (ESCAPE_KEYS.includes(String(key))) setidOfOpenDropdown(null); } - // NOTE: could the following be solved with context? - function clickOutsideNav(e) { - var thingYouClickedOn = e.target; - var areaToAvoid = document.getElementById("header-menu"); - if (!areaToAvoid.contains(thingYouClickedOn)) { - setidOfOpenDropdown(null); - } - } - - useEventListener( - "click", - clickOutsideNav, - document.querySelector("#global-nav-header") - ); - // --------------- - useEventListener("keydown", escapeDropdown); const options = { @@ -126,14 +111,16 @@ export function NavLinks({ aria-controls={`dropdown-${id}`} aria-expanded={id === idOfOpenDropdown ? true : false} > - {text}{" "} + {text} {id === idOfOpenDropdown ? ( ) : ( - + <> + + )} ) : ( diff --git a/src/Header/__snapshots__/Header.test.jsx.snap b/src/Header/__snapshots__/Header.test.jsx.snap index 72b86d44..cc2707f8 100644 --- a/src/Header/__snapshots__/Header.test.jsx.snap +++ b/src/Header/__snapshots__/Header.test.jsx.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`Header Matches snapshot 1`] = ` - + + `; diff --git a/src/Header/context/HeaderContext.js b/src/Header/context/HeaderContext.js new file mode 100644 index 00000000..f11536d4 --- /dev/null +++ b/src/Header/context/HeaderContext.js @@ -0,0 +1,36 @@ +import React, { useEffect, useState, createContext } from "react"; +import PropTypes from "prop-types"; + +const defaultValues = { accountMenuIsExpanded: false, idOfOpenDropdown: null }; + +export const HeaderContext = createContext(defaultValues); + +export const HeaderContextProvider = function ({ children }) { + const [idOfOpenDropdown, setidOfOpenDropdown] = useState(null); + const [accountMenuIsExpanded, setAccountMenuIsExpanded] = useState(false); + + const value = { + idOfOpenDropdown, + setidOfOpenDropdown, + accountMenuIsExpanded, + setAccountMenuIsExpanded, + }; + + useEffect(() => { + // we know both are now open + if (accountMenuIsExpanded && idOfOpenDropdown) { + setAccountMenuIsExpanded((prevState) => { + // if prevstate is not null, you know it was already open, therefore close it + if (prevState) return false; + }); + } + }, [accountMenuIsExpanded, idOfOpenDropdown]); + + return ( + {children} + ); +}; + +HeaderContextProvider.propTypes = { + children: PropTypes.node, +}; diff --git a/src/hooks/useClickOutside.js b/src/hooks/useClickOutside.js new file mode 100644 index 00000000..77d57602 --- /dev/null +++ b/src/hooks/useClickOutside.js @@ -0,0 +1,25 @@ +import { useEffect, useRef } from "react"; + +function useClickOutside(action) { + const ref = useRef(null); + + const handleClickOutside = (event) => { + // detects that the clicked element is not inside the referenced element + if (ref.current && !ref.current.contains(event.target)) { + action(true); + } + }; + + useEffect(() => { + document.addEventListener("click", handleClickOutside, true); + return () => { + document.removeEventListener("click", handleClickOutside, true); + }; + }, [handleClickOutside]); + + return { + ref, + }; +} + +export default useClickOutside; diff --git a/src/hooks/useClickOutside.test.jsx b/src/hooks/useClickOutside.test.jsx new file mode 100644 index 00000000..9d9e367a --- /dev/null +++ b/src/hooks/useClickOutside.test.jsx @@ -0,0 +1,36 @@ +import React from "react"; +import { mount } from "enzyme"; + +import useClickOutside from "./useClickOutside"; + +describe("useClickOutside hook", () => { + it("Returns true when the element clicked is outside the referenced element", () => { + const callbackFunction = jest.fn(); + + function MyWrapper() { + const { ref } = useClickOutside(callbackFunction); + + return ( + <> +
+ +
+ + + ); + } + + mount(, { attachTo: document.body }); + + const inside = document.getElementById("inside"); + inside.dispatchEvent(new Event("click")); + expect(callbackFunction).toHaveBeenCalledTimes(0); + + const outside = document.getElementById("outside"); + outside.dispatchEvent(new Event("click")); + expect(callbackFunction).toHaveBeenCalledTimes(1); + + outside.dispatchEvent(new Event("click")); + expect(callbackFunction).toHaveBeenCalledTimes(2); + }); +});