Skip to content

fix arc drawing when sweep >= 360#15042

Closed
nihgwu wants to merge 7 commits into
react:masterfrom
nihgwu:neo/fix-art
Closed

fix arc drawing when sweep >= 360#15042
nihgwu wants to merge 7 commits into
react:masterfrom
nihgwu:neo/fix-art

Conversation

@nihgwu

@nihgwu nihgwu commented Jul 17, 2017

Copy link
Copy Markdown
Contributor

I hit this issue in my own component https://github.com/nihgwu/react-native-pie, and find this https://stackoverflow.com/questions/19383842/weird-behaviour-in-drawing-a-ring-using-path-arcto-in-android, after this change, it looks exactly the same on both Android and iOS

@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 Jul 17, 2017
@nihgwu

nihgwu commented Jul 17, 2017

Copy link
Copy Markdown
Contributor Author

Sample code:

      <View style={styles.container}>
        <Pie
          radius={100}
          series={[10, 20, 30, -40]}
          colors={['red', 'lime', 'blue', 'yellow']} />
        <Pie
          radius={100}
          innerRadius={60}
          series={[10, 20, 30, 400]}
          colors={['#f00', '#0f0', '#00f', '#ff0']} />
        <View>
          <Pie
            radius={50}
            innerRadius={45}
            series={[100]}
            colors={['#f00']}
            backgroundColor='#ddd' />
          <View style={styles.gauge}>
            <Text style={styles.gaugeText}>100%</Text>
          </View>
        </View>
      </View>

After:
image

Before:
image

On iOS:
image

nihgwu added a commit to nihgwu/react-native-pie that referenced this pull request Jul 17, 2017
@nihgwu

nihgwu commented Jul 18, 2017

Copy link
Copy Markdown
Contributor Author

cc @javache @shergin

@pull-bot

pull-bot commented Aug 7, 2017

Copy link
Copy Markdown
Warnings
⚠️

📋 Test Plan - This PR appears to be missing a Test Plan.

@facebook-github-bot label Needs more information

Generated by 🚫 dangerJS

@shergin

shergin commented Aug 7, 2017

Copy link
Copy Markdown
Contributor

@nihgwu That's great!
One question: How the demo will look (on iOS) with 50% transparent colors? Could you please create a snack with your demo?

@nihgwu

nihgwu commented Aug 8, 2017

Copy link
Copy Markdown
Contributor Author

@shergin https://snack.expo.io/By3_faIPb
I changed the last value from 400 to 140 slightly, but the whole ring is still covered by the last color, seems SVG doesn't support transparent color?

@nihgwu

nihgwu commented Sep 8, 2017

Copy link
Copy Markdown
Contributor Author

@shergin Any thing should I do to make this be merged?

@shergin

shergin commented Sep 8, 2017

Copy link
Copy Markdown
Contributor

Android documentation said:

sweepAngle: Sweep angle (in degrees) measured clockwise, treated mod 360.

So, it means that it already does not support arcs "bigger" that 360 deg (but iOS does, I assume), so it means that this should not introduce a regression here.

Let's try to land it on Monday.

@nihgwu

nihgwu commented Sep 28, 2017

Copy link
Copy Markdown
Contributor Author

@shergin another release is coming, can we make this in?

@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Sep 28, 2017
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants