Skip to content

Conversation

@slorber
Copy link
Collaborator

@slorber slorber commented Jul 2, 2018

This permits to integrate more easily with other existing ScrollView wrappers/HOC's

To give more insight on my usecase, here's real code of my app (that will work once this PR is merged).
Note that I use glamorous ScrollView on purpose so in any case I need to pass it innerRef prop, not ref.

import React from 'react';
import { listenToKeyboardEvents } from 'react-native-keyboard-aware-scroll-view';
import { View, ScrollView } from 'glamorous-native';
import { ScrollIntoViewWrapper } from 'react-native-scroll-into-view';
import { AnimatedScrollView } from 'common/components/utils/GlamorousUtils';
import { compose } from 'recompose';

const makeKeyboardAware = Comp => {
  const debugOnAndroid = __DEV__ && true;
  const Wrapped = listenToKeyboardEvents(Comp);
  // Need to provide keyboard-aware hoc conf by props for now :(
  // See https://github.com/APSL/react-native-keyboard-aware-scroll-view/issues/272
  Wrapped.defaultProps = {
    refPropName: 'innerRef',
    enableOnAndroid: debugOnAndroid,
  };
  return Wrapped;
};

// We implement a custom scrollview because:
// - we need to ensure ios inputs remain visible after keyboard apparition (works by default on android)
// - we need equivalent of "element.scrollIntoView()" DOM function for some cases of the app
const CustomScrollView = compose(
  makeKeyboardAware,
  ScrollIntoViewWrapper({ refPropName: 'innerRef' }),
)(ScrollView);

export default CustomScrollView;

export const CustomScrollViewAnimated = compose(
  makeKeyboardAware,
  ScrollIntoViewWrapper({ refPropName: 'innerRef' }),
)(AnimatedScrollView);

@slorber slorber mentioned this pull request Jul 2, 2018
@slorber
Copy link
Collaborator Author

slorber commented Jul 11, 2018

@alvaromb can you please merge this? I've tested and it's working and is retrocompatible with current behavior

@alvaromb
Copy link
Collaborator

@slorber looking into this change atm.

@alvaromb
Copy link
Collaborator

I would be more comfortable by applying your change and moving from HOC to HOF (high order function).

How do you feel about that, @slorber?

@alvaromb
Copy link
Collaborator

If that's ok for you, I can provide HOF implementation today.

@slorber
Copy link
Collaborator Author

slorber commented Jul 12, 2018

not sure what you mean, but if you propose listenToKeyboardEvents(config)(Comp) instead (or in addition) of props like it is now, that would be fine for me yes

@alvaromb
Copy link
Collaborator

Yes! that's my proposal!

@slorber
Copy link
Collaborator Author

slorber commented Jul 12, 2018

great please ping me when you have a new version and I'll test it

@slorber
Copy link
Collaborator Author

slorber commented Jul 18, 2018

@alvaromb have you been able to work on this? I'm waiting for publishing my app ;)

I can work on a PR myself if you want, as long as you will merge it rapidly (otherwise I'd have to publish a fork)

@alvaromb
Copy link
Collaborator

@slorber I'll have a slot to implement that tomorrow. If you can send a PR before, I'll get that merged today.

@slorber
Copy link
Collaborator Author

slorber commented Jul 18, 2018

I can wait some days :)

Tell me if you didn't find time to do it tomorrow and I'll do it.
If so please do share your ideas, otherwise I'd implement something close to this API like the one I have done here: https://github.com/slorber/react-native-scroll-into-view/blob/master/scrollIntoView.js#L263

export const ScrollIntoViewWrapper = configOrComp => {
  if ( typeof configOrComp === "object" ) {
    return Comp => ScrollIntoViewWrapperHOC(Comp,configOrComp);
  }
  else {
    return ScrollIntoViewWrapperHOC(configOrComp);
  }
};

@slorber
Copy link
Collaborator Author

slorber commented Jul 30, 2018

@alvaromb unless you want to do it soon, I'm going to make another PR this week

@alvaromb
Copy link
Collaborator

@slorber we're in the middle of a very very important release and we do not have time right now to handle this reviews. I've added you as a collaborator, how do you feel about being collab & having release access?

@alvaromb
Copy link
Collaborator

alvaromb commented Jul 31, 2018

I think you should be able to merge PR's even if the master branch is protected. Ping me if not.

@slorber
Copy link
Collaborator Author

slorber commented Jul 31, 2018

Thanks, I'll make a PR for that probably today or tomorrow. If you want to to release i'm also @slorber on npm

@alvaromb
Copy link
Collaborator

I've also added you on npm. Please don't forget to write Release Notes: https://github.com/APSL/react-native-keyboard-aware-scroll-view/releases

@slorber
Copy link
Collaborator Author

slorber commented Aug 1, 2018

superseded by #288

@slorber slorber closed this Aug 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants