litho icon indicating copy to clipboard operation
litho copied to clipboard

Add View param to Visible and Invisible events.

Open vinc3m1 opened this issue 7 years ago • 7 comments

This may be something we keep as a branch but figured it's worth trying to upstream. In some cases we want a View during visible/invisible events, even if it's not the wrapping view, just to have some anchoring points and to get access to the hierarchy.

vinc3m1 avatar Oct 02 '18 19:10 vinc3m1

@vinc3m1 I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

facebook-github-bot avatar Feb 02 '19 18:02 facebook-github-bot

Hi @vinc3m1, what sort of use cases do you have here? Giving access to a View is always dangerous (e.g. risking people adding/removing Views in a Litho controlled hierarchy).. believe me it happened. Also, unless you really know the API, I think a dev would assume the received View is the one and only that wrapped the content where you set the visibility handler.

marco-cova avatar Feb 20 '19 14:02 marco-cova

Hi @vinc3m1. Is this PR still valid for you?

colriot avatar Jun 20 '19 14:06 colriot

Yep, we're still maintaining this patch and using it internally

vinc3m1 avatar Jun 20 '19 14:06 vinc3m1

Cool! @vinc3m1, can you then provide some info about the usecase? Which Marco asked for

colriot avatar Jun 21 '19 21:06 colriot

I believe we have some legacy APIs that take in a View to get approximate positioning information. I'm open to finding a way to make this clear that this isn't the same "view" that directly corresponds to the component.

vinc3m1 avatar Jul 09 '19 00:07 vinc3m1

I don't think we can directly add View here. What does approximate positioning information mean here? Is there any other information we can give off the View? Another thing we might consider is exposing the host LithoView on ComponentContext.

astreet avatar Jul 10 '19 21:07 astreet