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

montserrat #1784

Merged
merged 2 commits into from
Feb 1, 2021
Merged

montserrat #1784

merged 2 commits into from
Feb 1, 2021

Conversation

eatyourpeas
Copy link
Contributor

Hi. This is actually my first time contributing to an open source project that is not my own. So starting small, and hoping that I am not breaking some kind of etiquette. Forgive me if that is the case.

I am using Victory for a big project so getting to know it and enjoying the journey. I have been adding Montserrat to tooltips and finding that it breaks them so decided to add Montserrat sizes to your list in victory-core/victory-util/textsize.js using the bl.ocks.org suggestion.

Copy link
Contributor

@boygirl boygirl left a comment

Choose a reason for hiding this comment

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

This can be merged after a small syntax change

@@ -62,6 +62,10 @@ const fonts = {
widths: [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0.2359375,0.2234375,0.3921875,0.7125,0.49375,0.8859375,0.771875,0.2125,0.3078125,0.309375,0.375,0.4234375,0.234375,0.3125,0.234375,0.3,0.5828125,0.365625,0.434375,0.3921875,0.5234375,0.3984375,0.5125,0.4328125,0.46875,0.5125,0.234375,0.234375,0.515625,0.4234375,0.515625,0.340625,0.7609375,0.7359375,0.6359375,0.721875,0.8125,0.6375,0.5875,0.8078125,0.853125,0.4296875,0.503125,0.78125,0.609375,0.9609375,0.8515625,0.8140625,0.6125,0.8140625,0.71875,0.49375,0.7125,0.76875,0.771875,1.125,0.7765625,0.7734375,0.65625,0.321875,0.3078125,0.321875,0.3546875,0.5,0.3375,0.446875,0.5359375,0.45,0.5296875,0.4546875,0.425,0.4921875,0.54375,0.2671875,0.240625,0.5390625,0.25,0.815625,0.5375,0.5234375,0.5390625,0.5421875,0.365625,0.36875,0.35625,0.5171875,0.5015625,0.75,0.5,0.509375,0.44375,0.2421875,0.14375,0.2421875,0.35],
avg: 0.5116447368421051
},
"Montserrat": {
widths = [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0.2625,0.2609375,0.3734375,0.696875,0.615625,0.8296875,0.6703125,0.203125,0.3296875,0.3296875,0.3875,0.575,0.2125,0.3828125,0.2125,0.3953125,0.6625,0.3625,0.56875,0.5640625,0.6625,0.5671875,0.609375,0.5890625,0.6390625,0.609375,0.2125,0.2125,0.575,0.575,0.575,0.5671875,1.034375,0.7171875,0.7546875,0.7203125,0.8265625,0.6703125,0.634375,0.7734375,0.8140625,0.303125,0.5078125,0.7125,0.5890625,0.95625,0.8140625,0.8390625,0.71875,0.8390625,0.7234375,0.615625,0.575,0.7921875,0.6984375,1.1125,0.65625,0.6359375,0.6515625,0.31875,0.396875,0.31875,0.5765625,0.5,0.6,0.590625,0.678125,0.5640625,0.678125,0.6046875,0.375,0.6875,0.678125,0.2703125,0.365625,0.6015625,0.2703125,1.0625,0.678125,0.628125,0.678125,0.678125,0.4015625,0.4890625,0.40625,0.6734375,0.5421875,0.8796875,0.534375,0.5671875,0.5125,0.334375,0.2953125,0.334375,0.575],
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be object syntax like widths: [...], avg: num instead of width = [...], avg = num

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doh. Sorry should have checked closer. I will resubmit. Thank you!

@boygirl
Copy link
Contributor

boygirl commented Feb 1, 2021

@eatyourpeas Thank you for contributing! It's absolutely welcome. It looks like there's a small problem with your change, but if you push a fix, I'll get it merged and release a patch for you.

@eatyourpeas eatyourpeas closed this Feb 1, 2021
@eatyourpeas eatyourpeas reopened this Feb 1, 2021
@boygirl
Copy link
Contributor

boygirl commented Feb 1, 2021

🎉 I'll release a patch for you

@boygirl boygirl merged commit 1d3dd55 into FormidableLabs:main Feb 1, 2021
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.

2 participants