iOS: Replace NBSP with ZWSP to allow block rendering #520
Conversation
…d_of_blocks # Conflicts: # platforms/ios/lib/WysiwygComposer/Sources/HTMLParser/HTMLParser.swift
| for discardableTextRange in discardableTextRanges { | ||
| if discardableTextRange.upperBound <= attributedIndex { | ||
| actualIndex -= discardableTextRange.length | ||
| } | ||
| } |
There was a problem hiding this comment.
Think this raises a Swiftlint warning, I'm guessing we can e.g. write something like:
actualIndex -= discardableTextRanges
.filter { $0.upperBound <= attributedIndex }
.reduce(0) { $1 + $2.length }| for discardableTextRange in discardableTextRanges { | ||
| if discardableTextRange.location < actualIndex { | ||
| actualIndex += discardableTextRange.length | ||
| } | ||
| } |
There was a problem hiding this comment.
same here
but more generally I'm sure we can globally improve the code here, and perhaps merge list prefixes and discardable text into the same Array so we don't have multiple arrays that achieve same kind of computation
There was a problem hiding this comment.
I think there is actually something missing in my mapping, because when I delete the last character in the last newline, the rendering is lost for that line, I think that I need just like for the list, create some kind of exception based on the cursor position, because deleting the last character probably is also deleting the ZWSP character and removing the rendering, would be nice to be able to keep it, and removing it alongside the newline
There was a problem hiding this comment.
Yeah not sure what happens in that case, technically on backspace we do replace the HTML so we should be re-parsing and adding that ZWSP.
| enumerateTypedAttribute(.discardableText) { (discardable: Bool, range: NSRange, _) in | ||
| guard discardable == true else { return } | ||
|
|
||
| self.deleteCharacters(in: range) | ||
| let attributes = self.attributes(at: range.location, effectiveRange: nil) |
There was a problem hiding this comment.
we can directly get the backgroundStyle alone here and avoid looking up the entire dictionary ourselves
| var leadingDiscardableElement: DiscardableTextHTMLElement? | ||
| var trailingDiscardableElement: DiscardableTextHTMLElement? | ||
| var shouldReplaceNodes = false | ||
|
|
||
| if text.hasPrefix("\(Character.nbsp)") { | ||
| shouldReplaceNodes = true | ||
| text.removeFirst() | ||
| leadingDiscardableElement = createDiscardableElement() | ||
| } | ||
|
|
||
| if text.hasSuffix("\(Character.nbsp)") { | ||
| shouldReplaceNodes = true | ||
| text.removeLast() | ||
| trailingDiscardableElement = createDiscardableElement() | ||
| } | ||
|
|
||
| if shouldReplaceNodes { |
There was a problem hiding this comment.
I'm guessing this could be replaced by
let hasLeadingNbsp = text.hasPrefix(...)
let hasTrailingNbsp = text.hasSuffix(...)
if hasLeadingNbsp || hasTrailingNbsp {
removeAllChildNodes()
// Prepare nodes and push them, we only need to create the new nodes at this point
// Don't need to work with these optionals.
[...]
}| let newTextNode = DTTextHTMLElement() | ||
| newTextNode.inheritAttributes(from: self) | ||
| newTextNode.interpretAttributes() | ||
| newTextNode.setText(text) | ||
| addChildNode(newTextNode) |
There was a problem hiding this comment.
I think you might be able to re-use the same text node after detaching it instead of creating a new one (no need to re-inherit/interpret attributes then)
No description provided.