manufont / react-swipeable-bottom-sheet

A swipeable material's bottom sheet implementation, using react-swipeable-views
MIT License
131 stars 36 forks source link

Wrong scroll offset after closing #7

Closed gustavopch closed 7 years ago

gustavopch commented 7 years ago

Steps to reproduce:

Expected behavior:

The bottom sheet should automatically reset the scroll offset to zero when closed.

manufont commented 7 years ago

I don't think the plugin should handle the scrolling behavior of its content.

Here's how I would solve this issue (⚠️dirty code alert⚠️):

    render () {
        const contentStyle = {
            maxHeight: '100%',
            overflow: 'auto'
        };

        return (
            <div>
                <button onClick={() => this.openBottomSheet(true)}>
                    Open bottom sheet
                </button>
                <SwipeableBottomSheet
                    open={this.state.open}
                    onChange={this.openBottomSheet.bind(this)}
                    swipeableViewsProps={{
                        onTransitionEnd: () => {
                            if(!this.state.open) this.contentElt.scrollTop = 0;
                        }
                    }}
                >
                    <div style={contentStyle} ref={elt => this.contentElt = elt}>
                        <div style={{height: '2000px'}}/>
                        <button onClick={() => this.openBottomSheet(false)}>Close</button>
                    </div>
                </SwipeableBottomSheet>
            </div>
        );
    }

I use onTransitionEnd method instead of openBottomSheet so that content don't scroll during animation. Don't forget to set the reference on the content wrapping div.

gustavopch commented 7 years ago

I think I should have said that the issue happens when scrollOnWindowOverflow: true. When the bottom sheet is closed, it's expected that the little overflow appearing in the bottom of the screen will represent the top of the content. What's currently happening is: if you scroll and then close the bottom sheet, the overflow of the bottom sheet don't show the top of the content anymore.

I can't see how this could be the intended behavior.


After writing this I saw you removed the scrollOnWindowOverflow prop. Would you mind to explain why this should be handled outside of this component? I mean: in what kind of situation would someone ever want the bottom sheet overflowing the top of the screen without automatically becoming scrollable?

manufont commented 7 years ago

I removed the scrollOnWindowOverflow prop because I couldn't find a case where disable it would be useful. It's set to true by default (same as before), and you can still disable it by setting bodyStyle.

I can't see how this could be the intended behavior.

I can still add a prop to force scroll when closing. I'm not sure whether or not the scroll should be animated. No animation might be too abrupt, and an easing scroll might be too flashy.

manufont commented 7 years ago

I added scrollTopAtClose prop (default true) in the last release. You can see it in action here. It should solve the issue.

I also added marginTop prop which should be useful if you need to keep a navbar visible.

gustavopch commented 7 years ago

@manufont Oh, I thought you had given up on the scroll on window overflow thing. I don't know if you animated the scroll or not but it's feeling good to me. Thanks for your work on this!

happimaker commented 4 years ago

How to fix header so that I can control the header swipeable after I opened sheet and scrolled down?