bump react-modal 2.4.1 -> 3.0.4

[brucewpaul]

Issue:
react-modal needs to be ^3 as this is one of the dependencies that has a react peerdep. ^3 unlocks react16 as a peerdep

What I did

bumped react-modal

How to test

Is this testable with jest or storyshots?

Does this need a new example in the kitchen sink apps?

Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

[codecov]

Codecov Report

Merging #2098 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2098   +/-   ##
=======================================
  Coverage   22.16%   22.16%           
=======================================
  Files         268      268           
  Lines        5875     5875           
  Branches      706      718   +12     
=======================================
  Hits         1302     1302           
+ Misses       4040     4035    -5     
- Partials      533      538    +5

Impacted Files	Coverage Δ
app/react/src/server/utils.js	0% <0%> (-53.58%)	⬇️
addons/storyshots/src/require_context.js	0% <0%> (ø)	⬆️
addons/knobs/src/components/types/Color.js	8.1% <0%> (ø)	⬆️
addons/knobs/src/components/types/Number.js	8.06% <0%> (ø)	⬆️
lib/components/src/navigation/menu_link.js	0% <0%> (ø)	⬆️
app/react/src/client/preview/element_check.js	41.17% <0%> (ø)	⬆️
addons/knobs/src/components/types/Select.js	7.93% <0%> (ø)	⬆️
addons/info/src/components/PropTable.js	21.36% <0%> (ø)	⬆️
addons/info/src/components/Props.js	37.2% <0%> (ø)	⬆️
lib/codemod/src/transforms/update-addon-info.js	50% <0%> (ø)	⬆️
... and 19 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b74bac8...05c2d5a. Read the comment docs.

[Hypnosphi]

@ndelangen do we really need all three dependencies here (in lib/ui, app/react, and app/vue)? Looks like react-modal is only imported from lib/ui code

[Hypnosphi]

This mostly works, but there is one regression when comparing to master: when you open the search box, it isn't focused

Steps to reproduce:

yarn bootstrap --core
yarn start

Open http://localhost:9010/ and hit ⌘ ⇧ P. Expected: search box is focused. Actual: it isn't focused

[ndelangen]

@ndelangen do we really need all three dependencies here (in lib/ui, app/react, and app/vue)? Looks like react-modal is only imported from lib/ui code

Agree, looks like we can remove the dependency on react-modal from app/react and app/vue.

This PR is a good place to remove it in, Will you be able to do that @brucewpaul, or do you need one of use to do it? 👍

[brucewpaul]

I'll be able to get to this and I'll take a look at the bug you described as well.

Additionally, has there been discussion on adding react16 to peerDeps and what how you all want to get there?

[ndelangen]

I'll put React16 on the issue list for the roadmap meeting. #1724 is on react 16.

[Hypnosphi]

@ndelangen master is on React 16 as well

[ndelangen]

Any progress on this, Is there anything needed of me? @Hypnosphi

[Hypnosphi]

The only critical thing is this: #2098 (review)

[Hypnosphi]

I think I'll pick this one up

[Hypnosphi]

Superseded by #2238