Skip to content

insidetextorientation examples #27

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

Merged
merged 1 commit into from
Mar 1, 2020
Merged

insidetextorientation examples #27

merged 1 commit into from
Mar 1, 2020

Conversation

Mahdis-z
Copy link
Contributor

No description provided.

Copy link
Contributor

@jdamiba jdamiba left a comment

Choose a reason for hiding this comment

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

@Mahdis-z
Copy link
Contributor Author

Please use the same text as https://plot.ly/javascript/pie-charts/#control-text-orientation-inside-pie-chart-sectors
This explanation in the js tutorial sounds good, but we made some change when were writing py docs https://plot.ly/python/pie-charts/#controlling-text-orientation-inside-pie-sectors, so we can leave it as is?

@jdamiba
Copy link
Contributor

jdamiba commented Feb 28, 2020

@Mahdis-z Thanks for pointing out the Python version of these docs. While I can see the direction you guys were trying to go in when writing it, I still feel like the explanation in the js docs is much more clear for users, especially for those who don't already know what "tangential" or "radial" mean. I believe that it should be the standard language we use to describe this attribute across the R, JS, and Python docs.

However, I'll defer to @nicolaskruchten's judgement on whether or not this should block this PR from being merged.

Copy link
Contributor

@nicolaskruchten nicolaskruchten left a comment

Choose a reason for hiding this comment

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

le mieux est l'ennemi du bien in this case, let's get this merged. 💃

@jdamiba if you want to fix the Python and R versions together later that would be fine

@Mahdis-z Mahdis-z merged commit 681bb0e into master Mar 1, 2020
@Mahdis-z Mahdis-z deleted the text_orientation branch March 1, 2020 18:49
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.

3 participants