Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add onPressIn and onPressOut to View #1135

Closed
wants to merge 1 commit into from

Conversation

aschultz
Copy link
Member

@aschultz aschultz commented Aug 30, 2019

Surface the onPressIn and onPressOut props to go along with onPress. This provides consumers with a simple common pathway to update their visual styles in response to the stages of a mouse or touch press.

The implementation on native uses the existing touchable functions imported into View. On web, the Pointer Events API is used to provide native-similar behavior across both mouse and touch inputs -- this includes triggering onPressOut when a touch transitions to a scroll gesture or otherwise moves away from the initial press location.

Pointer Events are currently supported on all major browsers except Safari. An alternative could be built using Touch Events, but it would increase the tracking overhead and complexity of View, which is not ideal for a component that generally doesn't care about presses. Consumers who need support on Safari should be able to wrap View for their specific use case and leverage the onResponder* methods to add this tracking logic.

@erictraut
Copy link
Contributor

I'd really prefer not to add more interaction capabilities to RX.View. This class has gotten very bloated over time. It was never intended to be a class for user interactions. That's what RX.Button, RX.GestureView, etc. were designed for. The reason we started to add minimal support for interactions (e.g. onPress) in RX.View was to work around the fact that RX.Button couldn't be nested on the web because of limitations in HTML.

So my question is: Why is it necessary to add onPressIn and onPressOut capabilities to RX.View? Why doesn't RX.Button suffice here? It already supports this functionality.

If we do decide to include this functionality in RX.View, we'd need the PR to include documentation and unit tests (in RXTestApp) as well.

@aschultz
Copy link
Member Author

aschultz commented Sep 6, 2019

My particular use case here is enabling that nested button scenario to have better visual indicators when the button is down.

If View is going to support onPress, for any reason, I'd argue that it should also support onPressIn/onPressOut so that consumers can properly style it for those situations where onPress is needed. My attempt with this PR was to make that additional as minimalist as possible.

View presents more primitive interaction APIs than Button does at this point. Aside from mouse down/mouse up, the rest of Button's functionality (long press, disabled, hover, styling) appears to be replicable by wrapping View in another component. I agree that View shouldn't try to implement these directly, but having the necessary passthroughs provides a lot of flexibility to RXP consumers. They can benefit from some of the cross-platform logic done by RXP while still being able to customize the interactions and the UI shown in response to those interactions.

If there's need for a View that fits the original philosophy, perhaps creating a new SlimView that does the bare minimum would be appropriate?

@erictraut
Copy link
Contributor

OK, I buy that justification.

I see that you've already included documentation in your PR. Could you please add minimal test cases in RXPTestApp as well?

@aschultz
Copy link
Member Author

Will do. It might take me a few days to get around to it.

@deregtd
Copy link
Collaborator

deregtd commented Nov 11, 2019

@aschultz Poke :)

@erictraut
Copy link
Contributor

I'm going to close this PR since it hasn't received an update in a long time.

@erictraut erictraut closed this Jul 20, 2020
@fbartho
Copy link
Contributor

fbartho commented Jul 20, 2020

@erictraut I think this is mooted? Included if/when we port React-Native's new Pressable component?

Is that something we'd expose as a new top level component, or is that something we'd use to enhance RX.Button?

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.

5 participants