Swiping to open or close drawer don't work with preact.
only works swipe area's touch event.
In case of using react with same code, drawer works properly.
swipeable drawer can detect touch event and handle state.
Steps:
"@material-ui/core": "^4.8.1",
"next": "^9.1.7-canary.2",
"typescript": "^3.7.3",
"preact": "^10.1.1",
"preact-render-to-string": "^5.1.3",
sandbox (swipe is not work in sandbox editor)
https://codesandbox.io/s/exciting-worker-i67ug
[for reproduction] please swipe from right in this page
https://i67ug.sse.codesandbox.io/index
@tky5622 Could it be a bug in Preact? Unrelated to Material-UI?
@tky5622 Could it be a bug with Preact?
yes, this is a bug with preact (without preact , this code works)
and I think next.js is not relate to this bug.
Just in case, I'll create sandbox without next.js
When I said "with Preact", I meant as unrelated to Material-UI.
When I said "with Preact", I meant as unrelated to Material-UI.
I still don't know the exact cause of it.
However, it may be similar to the issue this pull request solved.
https://github.com/mui-org/material-ui/pull/18027
if synthetic event is used in swipe action, the problem is not with preact.
As far as I could dive into it, the issue comes from preact not firing the ref, in:
https://github.com/mui-org/material-ui/blob/a992b09f1c0c7d4e16ceb8b1982242c5a1dd6d7d/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js#L530
Without the reference to the paper DOM element, nothing works. I don't think that we can do more about it here. I would encourage you to report the issue on Preact side.
@oliviertassinari what's the reason for manually recreating object refs using findDOMNode? That seems like a strange hack, and likely the source of preactjs/preact#2256.
@developit I think that the motivation was to support the case when a developer provides a custom Backdrop that doesn't use React.forwardRef ref correctly. We have an open discussion about it on the React repo: https://github.com/facebook/react/pull/15091.
In the case of the Paper, it seems to be legacy, we can drop it :). Nice finding!
@tky5622 Do you want to solve this problem? I think that we could move forward with this simple fix :)
diff --git a/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js b/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js
index ddd7944600..0d59b86719 100644
--- a/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js
+++ b/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js
@@ -504,11 +504,6 @@ const SwipeableDrawer = React.forwardRef(function SwipeableDrawer(props, ref) {
backdropRef.current = ReactDOM.findDOMNode(instance);
}, []);
- const handlePaperRef = React.useCallback(instance => {
- // #StrictMode ready
- paperRef.current = ReactDOM.findDOMNode(instance);
- }, []);
-
return (
<React.Fragment>
<Drawer
@@ -527,7 +522,7 @@ const SwipeableDrawer = React.forwardRef(function SwipeableDrawer(props, ref) {
pointerEvents: variant === 'temporary' && !open ? 'none' : '',
...PaperProps.style,
},
- ref: handlePaperRef,
+ ref: paperRef,
}}
anchor={anchor}
transitionDuration={calculatedDurationRef.current || transitionDuration}
I will take give this a go as I am working on this feature lately and nobody has done anything on this yet. Check the PR I created:
Most helpful comment
@tky5622 Do you want to solve this problem? I think that we could move forward with this simple fix :)