fix safari dropdown not getting closed when click outside

[PiyushChandra17]

Fixes #2578

Changes:

Same as this, just added the same for CollectionListRow component

I have verified that this pull request:

• has no linting errors (npm run lint)
• has no test errors (npm run test)
• is from a uniquely-named feature branch and is up to date with the develop branch.
• is descriptively named and links to an issue number, i.e. Fixes #123

[PiyushChandra17]

@lindapaiste Can you please review this PR? As this issue is labeled as "Priority High", we need to fix this issue ASAP. Yes you were right that it's occuring on other components aswell ( fixed in this PR). There is one instance more it's occuring but it's happening on Chrome too, probably someone has raised an issue.

As discussed earlier, we need to stick with one approach of how we are handling outside click throughout our codebase, i think this is the way to go as it's more flexible and with less complications (might work for chrome issues aswell ). Thanks in advance.

[lindapaiste]

@lindapaiste Can you please review this PR? As this issue is labeled as "Priority High", we need to fix this issue ASAP. Yes you were right that it's occuring on other components aswell ( fixed in this PR). There is one instance more it's occuring but it's happening on Chrome too, probably someone has raised an issue.

As discussed earlier, we need to stick with one approach of how we are handling outside click throughout our codebase, i think this is the way to go as it's more flexible and with less complications (might work for chrome issues aswell ). Thanks in advance.

I really don't think that handling it piecemeal component by component is a scalable solution. That's how we get mistakes like the one in your picture where someone put in a menu and forgot to add click-outside handling (yes there is a PR it's #2525).

I did look into this more and the components which have the Safari problem don't have explicit outside click handling. They are relying solely on onFocus and onBlur events. The focus/blur stuff is good for keyboard Tab navigation but I guess Safari isn't firing the blur event when you click away like the other browsers do. I wonder if Safari also omits the blur when using Tab navigation? I have to check that.

I want us to have one place in the code where we write the logic for outside closing and then use that component or hook in other places. The useModalClose hook that I wrote ensures that every element which can be closed by clicking outside can also be closed via the Esc key. And then I used that in a DropdownMenu component, which also adds the focus/blur logic. Then the AssetList, SketchList, and CollectionListRow use the DropdownMenu (restyled as TableDropdown). They aren't declaring any of their own logic. They're just stating "this is a menu and here are the items in it".

Which is my long-winded way of saying that in my opinion the appropriate fix for this bug is to merge #2379.

@raclim: I know we've talked about giving me authority to approve my own PRs. In the meantime, can you just trust me on this and merge it for me? I know that when I'm refactoring things I'm changing a lot of code and that it's a lot for you to process. But I am writing good code and I'm fixing all sorts of issues like this that go on to reported by someone else months later.

[raclim]

@lindapaiste Sorry for the delayed response on this!

Skimming over it I think so far that makes sense to me and I'm down to merge in #2309 and #2379. There are a lot of changes happening and I could probably be better at getting to them more quickly 😆 but I definitely do trust your process and insight! I'm going to merge in #2649 first and deploy it since the p5.js version just got updated, and then I'll get to merging the other two in.

[raclim]

Closing with reasons outlined in #2578 (comment)!