Gaps show in list occasionally when dragging

[MossP]

Sometimes gaps show up in the list when dragging as mentioned in https://github.com/deanmcpherson/react-native-sortable-listview/issues/2

I believe the fix for the gaps is to update onPanRepsonderRelease()'s last state assignment from

this.state.active = false;
this.state.hovering = false;

to

if (this.props._legacySupport) {
  this.state.active = false;
  this.state.hovering = false;
} else {
  this.setState({
    active: false,
    hovering: false
  });
}

to match the assignment further up to trigger a refresh.

I can submit a PR but I've not had a chance to test this across iOS/Android versions yet.

[MossP]

That fix may not completely remedy the situation. I'll look into it more soon.

[smilingkylan]

I try to test this on my project as well, as I've been having the "missing" rows show up in my scrollable listview. Thanks for the heads up @MossP .

[smilingkylan]

@MossP In which file did you make that modification in the code?

[MossP]

@kylanhurt I added it here but I don't think it's a good idea and I haven't had time to go back and fix it properly yet.

[smilingkylan]

@MossP I doubt everyone is experiencing the issue, so it'd be interesting to see what specifically in our environment is causing the bug to manifest itself. If you think you might have a fix but don't want to submit a pull request yet, then you could fork the repo and give me access to it and I could do some testing.

I think the maintainer of the repo is looking for an exit, so even if you submit a pull request it might not make it into master / production for a while. I'm not knocking the maintainer, but we have to take the circumstances into consideration (and the project I'm using this repo on has a demo in a couple of months...)

[MossP]

@kylanhurt I don't have a fork. I only made that one change that I mentioned which I did by copying the source to a custom_modules folder and linking to that directly (to prevent npm from caching the original source). I have, however, since moved to another component which seems to not suffer from any of the bugs I experience with this one. The only downside being that I can't incorporate a swipe facility into the other component. https://github.com/gitim/react-native-sortable-list

[RUIFERNANDE5]

+1 If you drag any row to position [0] it creates a gap between 0 and 1. There are more cases when this happens but this was the most obvious.

"dependencies": {
    "react": "~15.4.1",
    "react-native": "0.42.0",
    "react-native-sortable-listview": "^0.2.2"
  },

---

import SortableListView from 'react-native-sortable-listview';

const RowComponent = (props) => {
    return (
        <TouchableHighlight underlayColor={'#eee'}
            style={{padding: 25, backgroundColor: "#F8F8F8", borderBottomWidth:1, borderColor: '#eee'}} {...props.sortHandlers}>
            <Text style={{fontSize: 20}}>{props.data.etc}</Text>
        </TouchableHighlight>
    )
}

export default class Sortable extends Component {
    constructor(props) {
        super(props);
        this.cards = {
            0: {text: 'world', etc: 'ok'},
            1: {text: 'are you', etc: 'abc'},
            2: {text: 123, etc: '3'},
            3: {text: 'is', etc: 'qeqeq'},
        }
        this.order = Object.keys(this.cards);
    }

    render() {
        return (
            <SortableListView
                style={{flex: 1, marginBottom: 0}}
                data={this.cards}
                order={this.order}
                onRowMoved={e => {
                    this.order.splice(e.to, 0,
                    this.order.splice(e.from, 1)[0]);
                    this.forceUpdate();
                  }}
                renderRow={row => <RowComponent data={row} />}
            />
        );
    }
}

[kennykswan]

@RUIFERNANDE5 I was able to solve the gap issue by modifying line 301:

let hoveringIndex = this.order[this.state.hovering] || this.state.hovering;

to:

let hoveringIndex = this.order[this.state.hovering];

[chetstone]

@kennykswan This looks promising. PR is welcome. Do you think you could modify the example app to demonstrate the bug?

[RUIFERNANDE5]

@kennykswan that worked, thanks! @chetstone Isn't my example app not clear enough?

[chetstone]

@RUIFERNANDE5 Sorry I haven't had time to look at your example in detail, but I'm sure there is no problem with it. What I'm asking is that it be added to the repo in your PR, either as a modification to the existing example or added in as a separate one. This will make it easier for me to test and merge the PR. It will also help with documentation and regression testing in the future.

Thanks so much for your help. I like your fix --- I love one line changes, especially those remove dodgy code!

[RUIFERNANDE5]

@chetstone I'd be very happy to do a PR but @kennykswan is the one who found the solution, and I didn't have any time to look up the rest of the code nor to test the modified code, sorry.

[smilingkylan]

Don't mean to light a fire under your butts, but this bug makes using this otherwise awesome plugin a big eye-sore. Not sure why my app seems to have the issue while some others do not, but getting it fixed would be greatly appreciated!

If you guys need any simple help I may be able to contribute, but I'm not familiar with Swift nor Java...

[MossP]

@kylanhurt The author and contributors of this component have given up their time to create this and allowed everyone to use it for free. Everyone understands that you are using it and have a deadline but their priorities will likely not align with yours. There are other solutions available that you could try but I don't think pushing them will aid your position.

I spent some time trying to fix the issue but couldn't consistently address it so I moved to something else given the limited time I had available. Solutions have been offered above that you can implement yourself very easily. Have you tried the suggested fixes?

If you guys need any simple help I may be able to contribute, but I'm not familiar with Swift nor Java...

See if the fixes work for you. Test them across platforms. If they work, fork and PR.

[adjustit]

I've just tested @kennykswan's fix successfully on iOS, I'm going to do a build for Android and see how it runs on there.

[chetstone]

I cannot replicate this problem using @RUIFERNANDE5 example code. (Which in fact is the same as the Sortable example but using a shorter list by default). Maybe I'm not looking in the right place for the problem. Index 0 is at the top of the list right? When I drag an item all the way to the top I'm supposed to see a slot open between the current items 0 and 1? I don't see it.

[chtrinh]

FYI: In android you probably want to leave this:

let hoveringIndex = this.order[this.state.hovering] || this.state.hovering;

or the drag item's Row will collapse to a height of 0.01 causing it to reflow and shift items below it to move up.

[nihgwu]

@chtrinh do you want to send a PR?

[chtrinh]

@nihgwu: i haven't validated it for iOS hence the lack of the PR. Just wanted to inform everyone about this. Not sure if everyone would like a Platform guard here. e.g.

if (Platform.OS === "android") {
  let hoveringIndex = this.order[this.state.hovering] || this.state.hovering;
}