Skip to content

[Android] Added backface-visibility support for android #6695

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.

Sign up for GitHub

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

Jump to bottom
Closed
kevinstumpf wants to merge 1 commit into from
Closed

[Android] Added backface-visibility support for android #6695

kevinstumpf wants to merge 1 commit into from
+62 −0

Conversation

@kevinstumpf
Copy link

@kevinstumpf kevinstumpf commented Mar 28, 2016

  • Motivation: Style backfaceVisibility was so far supported only on iOS
  • Test: I built react-native-flip-view and ensured that the back face of a rotated View is not visible anymore when its "backfaceVisibility" style is set to "hidden"
  • Implementation Details: Out of the box, Android doesn't support hiding a view's back face. The developed solution therefore takes into account the view's rotation. If its X or Y rotation is such that the back face is visible, it checks the backfaceVisibility setting. If it's set to "hidden", it sets the view's opacity to 0 -- otherwise it is set to the user defined value. Furthermore, this check is run whenever the view's opacity, transform matrix or backfaceVisibility changes.

Before:
y-flip-broken
x-flip-broken

After:
y-flip-working
x-flip-working

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 28, 2016

By analyzing the blame information on this pull request, we identified @mkonicek, @kmagiera and @jesseruder to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Mar 28, 2016
@kevinstumpf kevinstumpf mentioned this pull request Mar 28, 2016
Closed
@kevinstumpf kevinstumpf changed the title Added backface-visibility support for android [Android] Added backface-visibility support for android Mar 29, 2016
@ghost
Copy link

ghost commented Apr 25, 2016

@mkonicek would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

@mkonicek
Copy link
Contributor

mkonicek commented Apr 29, 2016

@kmagiera I think you were interested in reviewing this?

@ghost
Copy link

ghost commented May 29, 2016

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @jesseruder as a potential reviewer. Could you take a look please or cc someone with more context?

@felipemartim
Copy link

felipemartim commented Jun 29, 2016

any news?

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 12, 2016
@markzhai
Copy link

markzhai commented Aug 18, 2016

any updates?

@jasonfma
Copy link

jasonfma commented Aug 24, 2016

This would be awesome to get in.

@nihgwu nihgwu mentioned this pull request Sep 5, 2016
Closed
@mkonicek mkonicek assigned foghina and unassigned kmagiera Sep 9, 2016
@mkonicek
Copy link
Contributor

mkonicek commented Sep 9, 2016

Sorry about the delay! @foghina do you have any time to review? If not I'll review :)

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 9, 2016
foghina
foghina reviewed Sep 9, 2016
View reviewed changes
ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java
private static final String PROP_DECOMPOSED_MATRIX_ROTATE_X = "rotateX";
private static final String PROP_DECOMPOSED_MATRIX_ROTATE_Y = "rotateY";
private static final float FRONTFACE_VISIBILITY_MIN_ROTATION = -90.f;
private static final float FRONTFACE_VISIBILITY_MAX_ROTATION = 90.f;
Copy link
Contributor

@foghina foghina Sep 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: these don't really need to be constants. It makes the code less readable and we're not likely to change their value any time soon, and their use is pretty limited.

@foghina
Copy link
Contributor

foghina commented Sep 9, 2016 •
edited
Loading

Can't we use View.INVISIBLE instead of opacity? Then we don't need to remember the last explicitly set value etc.

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 9, 2016
@lacker
Copy link
Contributor

lacker commented Oct 24, 2016

Hey, sorry we didn't get to review this faster. It looks like this PR has been abandoned though because it's been over a month without an author-response. I'm going to close this pull request for now, but the functionality does look pretty cool so it would be nice to have it in and if you are still working on it then please feel free to reopen it.

@lacker lacker closed this Oct 24, 2016
@chirag04 chirag04 mentioned this pull request Nov 20, 2016
Closed
@karladler
Copy link

karladler commented Dec 11, 2016

Any updates on this?

@charpeni
Copy link
Contributor

charpeni commented Sep 21, 2017

A PR for backface-visibility is here #15970!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@foghina foghina

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.