sussol / react-native-generic-table-page

A customisable page with table of data, search bar, an optional extras
12 stars 9 forks source link

#624-Keyboard pop up disappearing issue fixed #15

Closed arjunSussol closed 5 years ago

arjunSussol commented 5 years ago

'react-native-keyboard-aware-scroll-view' package added to fix the keyboard issue when selecting fields on table to edit values.

Tested on both Android emulator and tablet and looks better than before.

andreievg commented 5 years ago

@arjunSussol, this looks good to me.

@edmofro do you still use this the module in Tupaia MediTrak ? I am not sure how react-native-keyboard-aware-scroll-view performs in IOS, but def an improvement in Android.

Thought to inform you before merging.

Basically the issue this is solving is, when a cell is focused, the table moves to the focused cell position and this for some reason dismisses the keyboard.

Anyways, this PR would solve two issues: https://github.com/sussol/mobile/issues/625, https://github.com/sussol/mobile/issues/624

edmofro commented 5 years ago

@andreievg thanks for checking in but nope, we don't use this any more :-)

edmofro commented 5 years ago

BTW that problem is probably because somewhere down the component tree there is a

keyboardDismissMode="on-drag"

We came across that recently. Seeing as this fix works, great, but you might also be able to hunt that down instead rather than requiring an additional dependency

andreievg commented 5 years ago

@edmofro, thank you for thoughtful comment.

@arjunSussol, since you've taken on this issue, can you spend a little bit of time and see if you can find the root cause as described by Edwin. But don't spend too long, as we've found a solution.

arjunSussol commented 5 years ago

@edmofro and @andreievg thanks for your thoughtful comment. I have looked at keyboardDismissMode="on-drag" and noticed that it is ScrollView props. However, we haven't used ScrollView and that props in our package.

So, adding react-native-keyboard-aware-scroll-view package would be beneficial.

Chris-Petty commented 5 years ago

@arjunSussol If I'm not mistaken, ListView extends ScrollView so should inherit keyboardDismissMode="on-drag".

edmofro commented 5 years ago

And if you're hunting for the use of ListView, it's inside the react-native-data-table library (also a sussol repo!)

Can pass through props from the parent though (react-native-generic-table-page)

It'd be worth just giving it a quick try...something like

<DataTable
        refCallback={(reference) => (this.dataTableRef = reference)}
        style={this.props.dataTableStyles.dataTable}
        listViewStyle={defaultStyles.listView}
        renderFooter={this.renderFooter}
        dataSource={this.state.dataSource}
        renderRow={this.renderRow}
        renderHeader={this.renderHeader}
        keyboardDismissMode={'none'}
      />

With the keyboard aware scroll view out of there obviously. My concern is that you're wrapping a scroll view (the ListView) in another scroll view (the KeyboardAwareScrollView), which seems like it's asking for trouble...

arjunSussol commented 5 years ago

@Chris-Petty and @edmofro Thanks for the guidance. I have applied keyboardDismissMode="none" in react-native-generic-table-page as suggested by @edmofro and presuming that passed props should receive via {...listViewProps} in react-native-data-table library. However, We could not achieve our goal as expected, the bug still exist with this approach.

@edmofro nested ScrollView is not a good idea, but keyboardDismissMode may not compatible with ListView.

edmofro commented 5 years ago

Cool - I'll stop holding this up!

andreievg commented 5 years ago

Thank you all for feedback comments and tests. Will merge this now, bump version and push to npm

Chris-Petty commented 5 years ago

FYI for @arjunSussol @andreievg @edmofro , it appears this has led to some nasty behaviour on large tables: https://github.com/openmsupply/mobile/issues/1179#issuecomment-528194777