Refactor ScrollCaptor and ScrollLock into hooks#4333
Refactor ScrollCaptor and ScrollLock into hooks#4333JedWatson merged 7 commits intoJedWatson:masterfrom
Conversation
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 4bcfb1b:
|
ebonow
left a comment
There was a problem hiding this comment.
Looks good. I like the changes though I would like to clear up some of the line spacing to make it more consistent, but that is minor compared to how much better this is to have the functionality better composed into hooks for separation of concerns without having to wrap in 2 HOC components
|
@Methuselah96 I was responding to a user about some of the menuPortal styling and props. I wanted to get your feedback based on what the documentation states and the fact that you happen to be working on this portion of code at the moment. Is the thought process reasonable and do you think it's something you would want to incorporate into this PR or another or not at all? |
|
@ebonow Thanks, I'll take a look when I get a chance. |
a251032 to
c28c930
Compare
c28c930 to
c8f9a59
Compare
c8f9a59 to
eb00026
Compare
This is a follow-up PR to #4330, and should not be merged until that one gets merged (since it includes the changes from #4330). This PR is not necessary in order to remove
findDOMNode, so it should not block a 4.0 publish.This PR removes the need force a re-render in
ScrollManagersince all of the logic is run as an effect.