Skip to content

Text Overflow Issue with Characters in Strings #544

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

Closed
Rahulhanje opened this issue Aug 8, 2024 · 19 comments
Closed

Text Overflow Issue with Characters in Strings #544

Rahulhanje opened this issue Aug 8, 2024 · 19 comments
Assignees
Labels

Comments

@Rahulhanje
Copy link
Contributor

Rahulhanje commented Aug 8, 2024

@SableRaf
I have encountered an issue where entering a string of text that exceeds the dimensions of the canvas results in the text being cut off or becoming unreadable. The problem arises when the length of the string surpasses the available canvas size.

example URL:https://processing.org/examples/charactersstrings.html

Screenshot 2024-08-08 150801

I’d like to work on this issue. Could @SableRaf please assign it to me? Also, I would appreciate any guidance on how to fix it.

@SableRaf
Copy link
Collaborator

SableRaf commented Aug 8, 2024

Hi @Rahulhanje,

Good catch! You are correct. The behavior of the live sketch does not match the behavior of the Processing code on the page. This is due to a discrepancy between the way that Processing and p5.js handle text wrapping.

In Processing, it is sufficient to call text(words, 50, 120, 540, 300); where the numbers define the boundaries of a rectangle container for the text. Text wrapping is active by default in that scenario.

However, in p5.js, you have to additionally call textWrap() to activate the wrapping. In this particular case, you should use the CHAR parameter, so textWrap(CHAR) (edit: It's a little more nuanced than that, see correction below)

To fix this, you will need to edit the corresponding livesketch.js file at https://github.com/processing/processing-website/blob/main/content/examples/Basics/Data/CharactersStrings/liveSketch.js

For more details about updating live examples, please see examples.md.

I'll assign this issue to you. Let me know if you still have questions and feel free to make a pull request for the change.

@SableRaf
Copy link
Collaborator

SableRaf commented Aug 8, 2024

Correction: the behavior difference is specific to character-wise wrapping. Word wrap DOES work in p5.js by default if you set boundaries for the text rectangle. However it does NOT wrap on characters.

@Rahulhanje
Copy link
Contributor Author

Thank you for assigning the issue to me I’ll start working on it and will need a bit of time to resolve it.

@Rahulhanje
Copy link
Contributor Author

Rahulhanje commented Aug 9, 2024

@SableRaf I Created a pull request on solving this issue can you please review the code and merge it

@Stefterv
Copy link
Contributor

Hi Rahulhanje,

I took a look at your pull request and have merged it into the website

@Rahulhanje
Copy link
Contributor Author

Thankyou @Stefterv this is my First open source Contribution

@Rahulhanje
Copy link
Contributor Author

I checked the website, and my change is visible. It's working perfectly fine

@SableRaf SableRaf reopened this Aug 12, 2024
@SableRaf
Copy link
Collaborator

Hi @Rahulhanje,

Thank you so much and congratulations on your first contribution! 🎉

I reopened this issue to mention that the livesketch.js code should replicate the original Processing sketch's behavior as closely as possible. This is important to avoid any confusion for users since the code displayed on the example page is the code of the Processing sketch. I have updated the documentation about examples to clarify this point.

Please open a new PR to revert the changes and only leave the s.textWrap(CHAR) modification.

Thank you again for your efforts! If you have any questions, feel free to ask.

@Rahulhanje
Copy link
Contributor Author

Rahulhanje commented Aug 13, 2024

@SableRaf
image
Sorry to bother you sir , but as mentioned in the documentation, I only edited livesketch.js. If I revert the code, except for
s.textWrap(CHAR), the error remains the same. If I've misunderstood the error, could you please explain exactly what I done
wrong and whate i need to change

@Rahulhanje
Copy link
Contributor Author

Rahulhanje commented Aug 13, 2024

@SableRaf
image

I reverted the changes except for s.textWrap(CHAR). Now, the characters vertically overflow the canvas. To fix this, I implemented dynamic height adjustment, but I suspect these code lines are not aligned with the expected behavior of the Processing sketch. Am I right?

@SableRaf
Copy link
Collaborator

SableRaf commented Aug 13, 2024

You are correct! The example code featured on the Character String example page doesn't resize the window. Therefore the livesketch.js version ported to p5.js should not either. The issue was that the text would overflow to the right of the canvas, as shown in your original screenshot. Therefore the fix involves wrapping on characters to match the default behavior of Processing.

Note: I described the character wrapping discrepancy between Processing and p5.js in more detail here: processing/p5.js#7163 (comment)

image

@Rahulhanje
Copy link
Contributor Author

Rahulhanje commented Aug 13, 2024

this means that i should not take care of vertical overflow of text right so it match the behavior of Processing.
am i correct sir
then i need to remove the dynamic height adjust feature right

@SableRaf
Copy link
Collaborator

Correct. The dynamic height adjustment was a clever addition but it doesn't match the Processing sketch so it needs to be removed. Thanks!

@Rahulhanje
Copy link
Contributor Author

Rahulhanje commented Aug 13, 2024

Ok sir thankyou for confirming i will make change and create a another pull request

I also noticed some typos below. Are those <br/> tags required? to display on screen or else i will correct that also and solve both in single PR

Screenshot 2024-08-13 222200

@SableRaf
Copy link
Collaborator

I also noticed some typos below. Are those <br/> tags required? to display on screen

This seems to be an issue on a small number of examples. Feel free to open an issue over at the https://github.com/processing/processing-examples/ repository.

@Rahulhanje
Copy link
Contributor Author

Okay, I created a new issue regarding that typo. Is it assignable to me? If possible, I request that it assigned to me.

@Rahulhanje
Copy link
Contributor Author

Now i can see the changes in deployed preview can please check the code and merge the pull request
image

@SableRaf
Copy link
Collaborator

Great! Thanks @Rahulhanje 🙏

@Rahulhanje
Copy link
Contributor Author

Rahulhanje commented Aug 14, 2024

Welcome @SableRaf
Thank you for assigning me with this issue. I appreciate the guidance provided, and I look forward to addressing the challenge and contributing to your organization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants