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

Native Border Radius support #6

Open
mrousavy opened this issue Jun 16, 2020 · 7 comments
Open

Native Border Radius support #6

mrousavy opened this issue Jun 16, 2020 · 7 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@mrousavy
Copy link
Owner

Currently, no border radius property is supported. You can achieve the same behaviour in react by setting overflow: hidden and then applying your borderRadius.

To natively support borderRadius the property group has to be exposed and set on the native views (UIImageView, ReactImageView).

For Android the official image view provides this borderRadius prop, see the react native source code, not sure why that doesn't work in my case since BlurhashImageView is a derived class (from ReactImageView)

On iOS, this looks really easy, see this stackoverflow question. Couldn't find the react native's Image component source code, if anyone can help me out here I'd appreciate it.

@mrousavy mrousavy added enhancement New feature or request help wanted Extra attention is needed labels Jun 16, 2020
@mrousavy
Copy link
Owner Author

When setting borderRadius right now, it does nothing on iOS but makes the Image disappear on Android..

@mrousavy mrousavy changed the title Border Radius support Native Border Radius support Jul 24, 2020
@mrousavy
Copy link
Owner Author

mrousavy commented Aug 29, 2020

Supported on iOS through 3f7d921, also enables shouldRasterize for the UIImageView's layer.

Since I moved from the ReactImageView to the native ImageView, Android still doesn't support it, no idea how to implement that on a layer level. If anyone has an idea, please comment or open a PR with a solution.

iOS Some wacky Android implementation I came up with

@mrousavy
Copy link
Owner Author

mrousavy commented Jan 6, 2021

On second thought, maybe it's not a good idea to try and support that natively. It's probably more efficient and less code to just let the containing view handle the clipping (border radius)

@gwenoleR
Copy link

gwenoleR commented Feb 18, 2021

Hi @mrousavy
It seems that in Android even if the containing View has the radius the Blurhash doesnt have it.. Or may be I made a mistake but work fine on iOS
image
Left: Android
Right: iOS

EDIT: My bad, on iOS I have a borderRadius params on the style props of the blurhash

EDIT2: I can reproduce the issue with this simple code :

<View style={{ width: 100, height: 100, borderRadius: 10, margin: 10 }}>
          <Blurhash
            blurhash="U56Td:M{00xvYjWCyEaKDgkX={M{Y8n$r;V?"
            style={{ width: 100, height: 100, borderRadius: 10 }}
          />
</View>

Output:
image
Left: Android, Right: iOS

EDIT 3: My bad, RTFM.
Adding overflow: hidden works 👌

<View
          style={{
            width: 100,
            height: 100,
            borderRadius: 10,
            margin: 10,
            overflow: 'hidden',
          }}
    >
          <Blurhash
            blurhash="U56Td:M{00xvYjWCyEaKDgkX={M{Y8n$r;V?"
            style={{ width: 100, height: 100, borderRadius: 10 }}
          />
</View>

Produce:
image

Sorry for that, and thanks for this good work ! 👏

@mrousavy
Copy link
Owner Author

@gwenoleR yes, overflow hidden is the key here. I'm still not sure if I want to natively support border radius or just leave it up to the user to wrap it in an overflow hidden view 🤔

@andreialecu
Copy link

One other interesting problem here is that if you set borderRadius to a large value on iOS it begins "inverting/collapsing" the border until it disappears.

This is with borderRadius: 100
Screenshot 2021-05-23 at 16 16 56

Originally I had borderRadius set to Number.MAX_SAFE_INTEGER and nothing was rendering.

@mrousavy
Copy link
Owner Author

@andreialecu let me know if #119 fixes this for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants