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

Improve breakLine handling #45

Closed
wants to merge 2 commits into from

Conversation

JodliDev
Copy link

Fix1:

When evaluating container tags (like div or p) they always have a break line before them. Which in general is what we want. But when the container tag is at the start, this would lead to an empty line before it.

So for example this
<div>This is one line</div>
is evaluated as:


This is one line

instead of (notice that there is to empty line above):

This is one line

Or more importantly this:

<ul>
<li><div>Item</div></li>
<li><div>Item</div></li>
</ul>

would create something like this:


  • Item

  • Item

Fix2:

Having something like this A<br><br><br>B would be evaluated as:

A

B

instead of:

A



B

I hope this is helpful for you :)

Disclaimer 1:
I tested my changes with data from the app, where the library is used and I am content that it works as expected.
But I did not run (or fix) any UnitTests because my knowledge of pure library development is limited.

Disclaimer 2:
In case Fix 2 breaks intended behaviour, you can just revert my change in this line:
https://github.com/JodliDev/ZMarkupParser/blob/2c6a74edc37678efd888d41c07bcdab7d0500bab/Sources/ZMarkupParser/Core/Processor/MarkupNSAttributedStringVisitor.swift#L23

- When block elements are at the start of a block
- Prevent reducing when multiple <br> exist in succession
@JodliDev
Copy link
Author

I added a fix for an unexpected source of crashes (when the HTML string is empty).

About the failing tests: At the moment I am on a tight schedule, so I dont really have the capacity to look into library development and figure out how to run tests (I assume its not that complicated, but I don't really have the head space right now 😅 ).
So please see this PR as a starting point for improvement. I you want to just "steal" my changes and close this PR, I am fine with that as well :)

@zhgchgli0718
Copy link
Member

will fix tests and review later, thanks.

@zhgchgli0718
Copy link
Member

I still working on it.

@zhgchgli0718
Copy link
Member

zhgchgli0718 commented Jan 14, 2024

Thank you for your contribution, but in the latest version 1.8.0, I have completely refactored the line break logic and implemented it to directly match the browser's line break behavior. Therefore, this pull request is no longer needed. Thank you again for your contribution.

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