add year popup goes away when clicking outside

[AyushSat]

This PR will allow the add year popup to be closed when anywhere else on the page is clicked.

Added event listeners to document to check where the clicks occur, if they are in the button toggle, else turn off the popup

Go to roadmap, press the add year to see popup, click outside the popup/button and see if it goes away.

[github-actions]

Deployed staging instance to https://staging-280.peterportal.org

[js0mmer]

Looks good. One minor gripe I have is that the effect seems to go off and add/remove the event listener every time the popup is toggled since show is in the dependency array. I know it will show a warning when it's not in the dependency but there's another workaround using a function as it mentions in the error message. This will fix the adding/removing the listener on every toggle.

I left a suggestion with this change.

Also what does the popupTarget ref do? I see that it references the div you added but I don't see it used anywhere except the dependency array. Is this intentional?

[js0mmer]

Suggested change

	useEffect(() => {
	const handleClick = (event: MouseEvent) => {
	if (target.current && target.current.contains(event.target as Node)) {
	setShow(!show);
	} else {
	setShow(false);
	}
	};
	const rootElement = document.getElementById("root");
	if(rootElement){
	rootElement.addEventListener('click', handleClick);
	}
	return () => {
	if(rootElement){
	rootElement.removeEventListener('click', handleClick);
	}
	};
	}, [popupTarget, target, show]);
	useEffect(() => {
	const handleClick = (event: MouseEvent) => {
	if (target.current && target.current.contains(event.target as Node)) {
	setShow(show => !show);
	} else {
	setShow(false);
	}
	};
	const rootElement = document.getElementById("root");
	if(rootElement){
	rootElement.addEventListener('click', handleClick);
	}
	return () => {
	if(rootElement){
	rootElement.removeEventListener('click', handleClick);
	}
	};
	}, [popupTarget, target]);

[ethanwong16]

LGTM! Take a quick look at my suggestion + Jacob's feedback 🔥

[ethanwong16]

I'd suggest adding a quick comment here explaining the gist of the change and what it's purpose is, so it's easier to maintain or apply to other areas in the future

[js0mmer]

Proper way of doing this would actually be using an OverlayTrigger. Edit: another link for getting it to close when clicking buttons

[js0mmer]

Will be redone using an OverlayTrigger.