microsoft / reactxp

Library for cross-platform app development.
https://microsoft.github.io/reactxp/
Other
8.29k stars 494 forks source link

ScrollView children behave differently on Web and Android #1131

Open berickson1 opened 5 years ago

berickson1 commented 5 years ago

On Android, ScrollView children grow (as seen in Button in screenshot) On Web, ScrollView children don't grow

This can be fixed by switching the style on the ScrollView div from block to flex. I don't see any immediate issues making this change - but posting here before raising a PR in case anyone has behaviour that relies on existing ScrollView behaviour.

import React, { Component } from "react";
import RX, { Button, View, Text, Styles, ScrollView } from "reactxp";

const buttonStyle = RX.Styles.createButtonStyle({ 
    borderRadius: 4,
    borderColor: 'grey',
    borderWidth: 1,
    padding: 4
})

interface Props {

}

interface State {

}

export default class App extends Component<Props, State> {
    render() {
        let items: JSX.Element[] = [];
        for (let i = 0; i < 100; i++) {
            items.push(<View><Text>{i}</Text></View>)
        }
        // Create a button that appears different on web vs Android
        return (
            <ScrollView>
                <View><Text>This is a Text in a View</Text></View>
                <Button style={ buttonStyle }><Text>This is a Button in a View</Text></Button>
                { items }
            </ScrollView>
        );
    }
}

See the button below in the screenshot

Screen Shot 2019-08-17 at 8 47 39 PM
erictraut commented 5 years ago

We've tried to move from block to flex for this div previously, and IIRC, it caused a bunch of problems. Sorry, I wish I remembered more of the details. It's been a couple of years.

I'm not opposed to this change, but it will require some significant testing to make sure it doesn't break things.

berickson1 commented 5 years ago

I don't remember the details either, just that it was left that way internationally. Looking at some internal projects, the scroll view is almost always has an inner containing div to create the behavior show in RN above. Admittedly I don't have a great surface area to test this in beyond a personal project that I'm migrating to RX at the moment. I'd like to slot this into our 2.0 rc release and watch for feedback.

erictraut commented 5 years ago

Yeah, I agree it makes sense to make this change in 2.0 rc.

berickson1 commented 5 years ago

This is a little more tricky than I initially suspected - Applying display: flex to the div causes some controls to shrink. This can be worked around by wrapping the children in another div (much like React Native does). I'm playing around with this and it looks promising, but will have to do a little more testing before posting a PR.

An additional piece that I noticed - the vertical attribute doesn't work on any platform beyond web.

erictraut commented 4 years ago

@berickson1, what's the latest on this one?

berickson1 commented 4 years ago

I ran into a few issues with this initially regarding switching the wrapper to block from flex and then lost the branch when transitioning to a new machine. I worked around this in my own project by adding a wrapper view inside the scrollview and I haven't had a chance to pick this back up for RX

fbartho commented 4 years ago

@bericson1 @erictraut -- I've been noticing this problem myself recently, and in general it feels like the default of flexDirection is different on Web vs Native-iOS/Android with ReactXP.

What should we do here? I'd be happy to do the implementation if I get a little bit more precision on what you recommend here.