-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Revert of PR #4712, restores text wrapping behavior #5093
Conversation
🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me! thank you @lawreka!!
@all-contributors please add @lawreka for code and ideas |
I've put up a pull request to add @lawreka! 🎉 |
src/core/p5.Renderer.js
Outdated
let testWidth; | ||
let words; | ||
let totalHeight; | ||
let shiftedY; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your review @outofambit, but I think I made a mistake here... adding this shiftedY variable was part of the original PR that I'm reverting part of, but doesn't have to do with my fix, so I'm just going to push a small change to not touch this before this gets merged :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh i just did a line by line comparison of this against the original PR :)
thanks for catching that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the same and didn't see it until after your approval either 😅 just pushed the fix, should be good 2 go now
Resolves #5081
Changes:
Reverts PR #4712 and restores text wrapping behavior as seen in version 1.1.9.
Screenshots of the change:
Current rendering of 'Lorem ipsum dolor sit amet, consectetur adipiscing elit' in 100, 100 canvas:
Fixed:
Note this also re-introduces Issue #4652, future improvements should change how single words that exceed the length of a line can be handled by hyphenation or not.
PR Checklist
npm run lint
passesInline documentation and unit tests unchanged