PDF4Teachers icon indicating copy to clipboard operation
PDF4Teachers copied to clipboard

refactor(TextWrapper): simplify and optimize text wrapping logic

Open arturhg opened this issue 2 years ago • 3 comments

Streamline the text wrapping logic in the TextWrapper class by:
    - Combining fillLineWithWord and fillLineWithChar methods into a single fillLine method with a 'splitByWords' parameter, removing duplicate code for handling word and character wrapping
    - Replacing the usage of ScratchText with Text from JavaFX to measure the text width
    - Using StringBuilder for better performance when building wrapped lines and appending lines, instead of concatenating strings
    - Replacing the custom test method with the more descriptive exceedsMaxWidth method for determining if a line's width exceeds the maximum allowed
Update the TextElement class to use the new hasWrapped() method in TextWrapper:
    - Replace the previous call to doHasWrapped() with hasWrapped() to reflect the updated method name
Remove unused imports from TextWrapper:
    - Remove the import of ScratchText, as it is no longer used in the class
    - Remove the import of Pattern, as the regular expression functionality is no longer required in the updated implementation
Update the wrapFirstLine and wrap methods in TextWrapper to use the new wrapTextLineByWordsOrChars method, which consolidates the logic for wrapping text by words or characters based on whether the first word exceeds the maximum width

arturhg avatar May 07 '23 11:05 arturhg

Hi, @ClementGre. Can this PR be merged?

arturhg avatar Jun 29 '23 21:06 arturhg

I'm just unsure whether switching from ScratchText to Text for checking text width would cause issues. As I said, the Text class has styling attributes that prevent setting a custom font to it. Have you noticed any difference in behaviour?

ClementGre avatar Jun 30 '23 07:06 ClementGre

Can you change back Text class to ScratchText? Then this PR will be ready to be merged.

ClementGre avatar May 05 '24 07:05 ClementGre

Hi @ClementGre.

I changed back Text class to ScratchText.

Thank you!

arturhg avatar Jul 20 '24 11:07 arturhg

@ClementGre

I resolved the merge conflict.

arturhg avatar Jul 20 '24 13:07 arturhg

@arturhg thank you, your code works well!

ClementGre avatar Jul 20 '24 14:07 ClementGre