constelation / monorepo

Mono repo for constelation's Components, functions, and CONSTANTS
https://constelation.github.io/monorepo/
MIT License
12 stars 3 forks source link

<Event_> can't wrap a composite component #28

Closed thomasbruketta closed 7 years ago

thomasbruketta commented 7 years ago

Currently if you try to wrap a composite component with <Event_> component no event is fired.

ex, the following won't work:

<Event_
  onPress={this.handlePress}
>
    <ListItem>{this.props.children}</ListItem>
</Event_>
thomasbruketta commented 7 years ago

You can solve by either wrapping the composite component in a constelation <View> or by using TouchableOpacity instead. My current solve is to use TouchableOpacity instead.

kylpo commented 7 years ago

Thanks for making a ticket.

Does ListItem have a constelation-View as its outermost wrapper?

Also, remember that wrapping ListItem in a View is exactly what TouchableOpacity does. https://github.com/facebook/react-native/blob/master/Libraries/Components/Touchable/TouchableOpacity.js#L171

thomasbruketta commented 7 years ago

Here is ListItem:

// @flow weak
import React, { PropTypes } from 'react';

import View from 'constelation-View';
import Style_ from 'constelation-Style_';

export default class  extends React.Component {
  static propTypes = {
    borderBottom: PropTypes.bool,
  };

  static defaultProps = {
    borderBottom: false,
  };

  render() {
    return (
      <Style_
        borderTopWidth={1}
        borderBottomWidth={this.props.borderBottom ? 1 : 0}
        borderColor={COLORS.BORDER_LIGHT}
        backgroundColor={COLORS.WHITE}
      >
        <View
          height={60}
          paddingHorizontal={28}
          justify={'center'}
        >
          {this.props.children}
        </View>
      </Style_>
    );
  }
}
thomasbruketta commented 7 years ago

I first I thought the Style component wasn't passing props but I removed the Style and it still didn't work

kylpo commented 7 years ago

Cool, thanks! Looks like a bug.

thomasbruketta commented 7 years ago

Yeah I'll try to loop back to this early next week after we get done with our current sprint of work.

kylpo commented 7 years ago

Yo, so this is because the event props are passed in to the ListItem, but they aren't being passed in to ListItem's View. The same problem exists if you replace Event_ with TouchableWithoutFeedback.

SO, we need to wrap composite components with a View, like TouchableOpacity does. If we do this enough, we can make an Event or EventView component that automatically wraps with a View.