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

Image recommendations - Add utility class to style HTML #4759

Merged
merged 11 commits into from Feb 26, 2024

Conversation

tonisevener
Copy link
Collaborator

@tonisevener tonisevener commented Feb 22, 2024

Phabricator:
https://phabricator.wikimedia.org/T356816

Notes

This PR creates a utility class for converting html strings into NSAttributedString and AttributedString. We need the AttributedString conversion to support attributed SwiftUI text views in the Components framework.

I believe I have covered all of the major needs from our old class in String+Html.swift, though note that I didn't strictly follow it. This utility supports:

  • Bold
  • Italics
  • BoldItalics
  • Links
  • Lists
  • Superscript
  • Subscript
  • Underline
  • Strikethrough

I created some dummy UI to see both UI frameworks in action. Followup PRs for this task will be reusing the article intro view from the T347121/prototype, and connecting it to live data.

Test Steps

  1. On Staging or Experimental scheme, launch app.
  2. Tap Notifications Center bell icon.
  3. The top view is SwiftUI, and the bottom view is UIKit. Confirm styles look correct. Feel free to play with the text in WKImageRecommendationsViewController.swift to test different stylings.

Screenshots

screenshot2

@tonisevener tonisevener requested review from a team and staykids and removed request for a team February 22, 2024 16:07
Copy link
Contributor

@staykids staykids left a comment

Choose a reason for hiding this comment

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

Working nicely! Some minor optional stuff below from my first pass for future proofing if it also resonates with you, but happy to continue reviewing and approve as-is.

public static func nsAttributedStringFromHtml(_ html: String, styles: Styles) throws -> NSAttributedString {
let attributedString = NSMutableAttributedString(string: html)
let paragraphStyle = NSMutableParagraphStyle()
paragraphStyle.lineSpacing = 3
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about propagating this value through Styles as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done! One thing that's still a bit awkward here is that SwiftUI adds line spacing as a modifier, outside of the utility. I couldn't find any line spacing key while making AttributedString. I guess we could consider making a WKHtmlText component or something, just for SwiftUI, with line spacing applied within so that we don't have to remember to add this modifier every time. Let me know what you think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah a component that itself encapsulates those custom modifiers makes sense in the future to me too.

Components/Sources/Components/Utility/HTMLUtils.swift Outdated Show resolved Hide resolved
@staykids staykids removed their assignment Feb 24, 2024
@staykids staykids self-assigned this Feb 26, 2024
@staykids staykids merged commit 85f368a into main Feb 26, 2024
4 checks passed
@staykids staykids deleted the image-rec-intro-view-1 branch February 26, 2024 19:56
@staykids staykids removed their assignment Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants