search

davidtheclark / react-aria-modal

A fully accessible React modal built according WAI-ARIA Authoring Practices
http://davidtheclark.github.io/react-aria-modal/demo/
MIT License
1.03k stars 96 forks source link

Hotfix/update react version rename unsafe lifecycles #95

Closed resource11 closed 2 years ago

resource11 commented 4 years ago

Hey @davidtheclark! I did work on bumping some of the dependencies for this package (and did the same for focus-trap-react and react-displace to remove unsafe lifecycles from this lovely modal component. Do you mind taking a look?

davidtheclark commented 4 years ago

cc @ArfatSalman @rahildar, who are taking over maintenance.

ArfatSalman commented 4 years ago

@davidtheclark @resource11 Can we put check for titleId and titleText in the constructor of Modal? This way we can remove the componentWillMount call altogether.

resource11 commented 4 years ago

Moved the check down to the render() method. We'll still need to work on lifecycle methods for two dependencies (react-displace and focus-trap-react), yes, @ArfatSalman?

ArfatSalman commented 4 years ago

We'll still need to work on lifecycle methods for two dependencies (react-displace and focus-trap-react), yes, @ArfatSalman?

focus-trap-react does not have a componentWillMount and we can safely put the componentWillMount of react-displace in the constructor. @davidtheclark @resource11

wldcordeiro commented 4 years ago

FYI I'm gonna try moving the work of updating the dependencies to another PR so you can isolate the lifecycle work here.

resource11 commented 4 years ago

I had already started the other dependency work in those libraries, @wldcordeiro, and was submitting PRs for those. If you want to take them on instead, I'm good with that!

toolness commented 4 years ago

Hello! It seems the last activity on this PR was over a month ago--is there any way I can help move it forward?

AElmoznino commented 4 years ago

Hey y'all! Good work! Any progress on this? Would be awesome to get the warnings gone 😄

resource11 commented 3 years ago

My apologies for the delay, @ArfatSalman! I got sidetracked with a Covid-inspired layoff and a job search. Made the requested changes and also bumped up some of the dependencies.

marlomgirardi commented 3 years ago

My apologies for the delay, @ArfatSalman! I got sidetracked with a Covid-inspired layoff and a job search. Made the requested changes and also bumped up some of the dependencies.

I'm not sure about mixing dependency updates and react unsafe method fix in the same PR. Other than that, LGTM

marlomgirardi commented 2 years ago

I've done a fix for the componentDidMount at #121. I'm closing this issue but feel free to add separated MR.