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

HTML builder improvement #320

Merged
merged 11 commits into from
Jul 10, 2018
Merged

HTML builder improvement #320

merged 11 commits into from
Jul 10, 2018

Conversation

Ostrenkiy
Copy link
Contributor

Задача: #APPS-1955

Описание:
Избавились от левых файлов с HTML парсингом. Запилили HTMLProcessor, который подгружает все нужные скрипты и стили в нужные места, а также заменяет все, что не нужно в html-е на все, что нужно. Теперь для процессинга HTML не нужно знать ширину содержащей вьюшки, а стили не кривые и стыдные, а в css файлике.

@Ostrenkiy Ostrenkiy added this to the 1.63 milestone Jul 9, 2018
@Ostrenkiy Ostrenkiy self-assigned this Jul 9, 2018
@Ostrenkiy Ostrenkiy requested a review from kvld July 9, 2018 19:53
Copy link
Contributor

@kvld kvld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

На самом деле, этот код прямо просит, чтобы его покрыли тестами :trollface:

@@ -30,8 +30,7 @@ class CellWebViewHelper: NSObject {

//Method sets text and returns the method which returns current cell height according to the webview content height
func setTextWithTeX(_ text: String, color: UIColor = UIColor.mainText) {
let scriptsString = "\(Scripts.localTexScript)\(Scripts.mathJaxFinishedScript)"
let html = HTMLBuilder.sharedBuilder.buildHTMLStringWith(head: scriptsString, body: text, textColor: color)
let html = HTMLProcessor.shared.process(htmlString: text, head: Scripts.mathJaxFinishedScript, textColor: color)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мне кажется, что лучше было бы сделать процессор от обычной строки. А добавление скриптов в head/body и окрашивание в цвет (по сути тоже js скрипт) можно делать отдельно. В этом плане мне нравится Atributika. То есть, интерфейс примерно такой:

let processor = HTMLProcessor(html: text)
let processedHtml = processor
    .inject(.maxJax, where: .head)
    .inject(.textColor, where: .body)
    .html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Выглядит хорошо, да

case .metaViewport:
return Scripts.metaViewport
case .localTex:
return Scripts.localTexScript
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scripts.localTexScript -> Scripts.localTex
и тд

switch self {
case .audio:
return Scripts.audioTagWrapperInit
case .textColor(color: let color):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно просто писать case .textColor(let color) же 🤔

var body = body
body = fixProtocolRelativeURLs(html: body)

var links = HTMLParsingUtil.getAllLinksWithText(body).map({return $0.link})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var links = HTMLParsingUtil.getAllLinksWithText(body).map { $0.link }

let script = loadScriptWithKey(textColorScriptKey)
return script.replacingOccurrences(of: "######", with: "#\(color.hexString)")
}

fileprivate static var mathJaxLocalPathScript: String {
// let path = NSBundle.mainBundle().pathForResource("MathJax", ofType: "js", inDirectory: "MathJax")!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scripts может быть тоже почистить от всякого мусора?

@Ostrenkiy Ostrenkiy merged commit e2f37d4 into dev Jul 10, 2018
@Ostrenkiy Ostrenkiy deleted the feature/html-builder-improvement branch July 10, 2018 19:56
@kvld kvld mentioned this pull request Jul 11, 2018
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