Loading an image with Styler #258
Conversation
MrWoWander
changed the title
Codecov Report
@@ Coverage Diff @@
## master #258 +/- ##
==========================================
- Coverage 91.31% 84.17% -7.14%
==========================================
Files 60 61 +1
Lines 1059 1150 +91
==========================================
+ Hits 967 968 +1
- Misses 92 182 +90
Continue to review full report at Codecov.
|
|
I cannot yet add testing, as Codecov asks for it, since I don’t understand how testing works in your project.
|
|
Hi @MrWoWander , thanks for the pull request! I'll take a look at this soon and get back to you. |
|
And the question arose: is it necessary to make a function that will automatically resize the image if it is larger than the device's resolution in width? |
|
For iOS, I did this: extension UIImage {
func scalePreservingAspectRatio(width: CGFloat) -> UIImage {
let widthRatio = size.width / width
let heightRatio = size.height / widthRatio
// Compute the new image size that preserves aspect ratio
let scaledImageSize = CGSize(
width: width,
height: heightRatio
)
// Draw and return the resized UIImage
let renderer = UIGraphicsImageRenderer(
size: scaledImageSize
)
let scaledImage = renderer.image { _ in
self.draw(in: CGRect(
origin: .zero,
size: scaledImageSize
))
}
return scaledImage
}
}I personally use Down in conjunction with SwiftUI and while working in VStack, the picture crawls out of the edges of the screen. This method allows you to solve this problem. |
|
I also noticed that GIF images don't work. They are static. I'll try to come up with some solution |
|
@MrWoWander sorry for the delay in reviewing this, it's been a busy past week. I'm planning to take a look at tonight. |
| URLSession.shared.dataTask(with: url) { data, _, _ in | ||
| if let data = data { |
There was a problem hiding this comment.
A couple things:
- What happens when the url doesn't point to an image?
- What happens when there is a network error?
- I think it's important to make image fetching an optional behavior (preferably opt in) since users of the library may not want to automatically fetch data from unknown urls. This could be achieved with a simple flag in the styler configuration.
I think it would be good to limit the size of the image to fit within the view that contains the string.
I use snapshot tests to assert the styling methods. In the case of images, you could probably add a test image and get the url of that resource. I think it would be good to have several tests to assert various cases of images, such as small images, large images, and invalid urls. |
|
@johnxnguyen, ok, I'll try in the coming days to think about replacing semaphore and handling URL requests, with the option to prohibit downloading and downloading from local paths I will also add a reduction in the size of the images |
|
@MrWoWander let me know if you need help, I'm happy to discuss ideas and problems together! |
|
@johnxnguyen Please check it out. If it works the way you wanted, then I will already make the error handler and the rest of the functionality that we discussed |
|
In some cases, I did not have time to download images in the SwiftUI view, so I added a delegate that allows me to update the view |
| func textAttachmentDidLoadImage(textAttachment: AsyncImageLoad, displaySizeChanged: Bool) | ||
| } | ||
|
|
||
| final public class AsyncImageLoad: NSTextAttachment |
There was a problem hiding this comment.
Does this need to be public?
There was a problem hiding this comment.
Name suggestion: AsyncImageAttachment
|
|
||
| final public class AsyncImageLoad: NSTextAttachment | ||
| { | ||
| public var imageURL: URL? |
There was a problem hiding this comment.
Does this need to be optional?
|
|
||
| public var maximumDisplayWidth: CGFloat? | ||
|
|
||
| public var delegate: AsyncImageLoadDelegate? |
There was a problem hiding this comment.
Shouldn't this delegate be ��weak?
| { | ||
| let imageSize = image.size | ||
|
|
||
| if self.displaySize == nil |
There was a problem hiding this comment.
Is the purpose of displaySize to allow an explicit size to be set for the image, rather than it's actual size?
| // find character range for this attachment | ||
| let range = NSRange(location: 0, length: attributedString.length) | ||
|
|
||
| var refreshRanges = [NSRange]() |
There was a problem hiding this comment.
Would we ever expect to find more than one range for a single attachment?
| } | ||
|
|
||
| /// Trigger a relayout for an attachment | ||
| public func setNeedsLayout(forAttachment attachment: NSTextAttachment) |
There was a problem hiding this comment.
Does this need to be public?
| @@ -40,10 +40,12 @@ open class DownStyler: Styler { | |||
| .font: fonts.listItemPrefix, | |||
| .foregroundColor: colors.listItemPrefix] | |||
| } | |||
|
|
|||
| private var delegate: AsyncImageLoadDelegate? | |||
There was a problem hiding this comment.
Name suggestion: asyncImageAttachmentDelegate
| let image1Attachment = AsyncImageLoad(imageURL: url, delegate: delegate) | ||
| let image1String = NSAttributedString(attachment: image1Attachment) |
There was a problem hiding this comment.
Name suggestions: attachment, and attachmentString
| guard let url = url, | ||
| let urlImg = URL(string: url) else { return } |
There was a problem hiding this comment.
Suggestion:
guard let imageURL = url.flatMap(URL.init) else { return }
Added the ability to download an image from the markdown using the Styler class