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

[Feature] - Support for image resizing inside Markdown text #376

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

paul1893
Copy link

@paul1893 paul1893 commented Dec 20, 2024

Hello 👋,

I'm not sure if this has been discussed already, as I didn't see anything directly related in the "Issues" section.

In some forms of markdown, you can specify the size of an image (like in GitLab, for example) in this way:

![Foo image](https://foo/bar.png){width=50px}

With the following variations:

  • {width=50px}
  • {width=50px height=50px}
  • {height=50px width=50px}
  • {height=50px}
Capture d’écran 2024-12-20 à 23 57 49

I'm not an expert in markdown, so I'm unsure if resizing an image follows a specific convention or if it even makes sense to be supported directly by Swift-Markdown-UI. Perhaps the library already supports it? <img width="600" alt="x" src="y" /> doesn't seem to be interpreted for example.

Feel free to close this PR if it doesn't make sense or need to be supported in another way.

Regardless, great library 👏 Thanks.

@paul1893 paul1893 force-pushed the feature/support-image-resizing-suffix branch from d99a1fa to 3b4c9e2 Compare December 20, 2024 23:15
Comment on lines 109 to 121
if let size {
if let width = size.width, let height = size.height {
content.frame(width: width, height: height)
} else if let width = size.width, size.height == nil {
content.frame(width: width)
} else if let height = size.height, size.width == nil {
content.frame(height: height)
} else {
content
}
} else {
content
}

Choose a reason for hiding this comment

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

The width and height parameters of the frame ViewModifier are optional, so it should be possible to do just:

content.frame(width: size?.width, height: size?.height)

Copy link
Author

Choose a reason for hiding this comment

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

You're absolutely right. I'm making the change.

Comment on lines 53 to 56
/// - {width=50px}
/// - {height=50px}
/// - {width=50px height=100px}
/// - {height=50px width=100px}

Choose a reason for hiding this comment

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

It would be awesome if it also supported proportional width and height
e.g. {width=50%}

Copy link
Author

Choose a reason for hiding this comment

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

Yes I thought about it too. Can be quickly achieve for iOS 17+ with the new containerRelativeFrame modifier for horizontal configuration.

But I'm not sure of what 50% height would mean in the context of a vertical scrolling view. Does it mean 50% of the scrollview content, 50% of the nearest block where the content is, ... Depending on the answer could be harder to implement.

I've draft a commit to demonstrate horizontal relative width handling.

This is a cat with {width=50%}
relative-size

Copy link

@cernym46 cernym46 Jan 11, 2025

Choose a reason for hiding this comment

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

I wouldn't support relative height in the vertically scrollable ScrollView and relative width in the horizontally scrollable ScrollView. GeometryReader also "doesn't work" in these cases and returns a size equal to 10, but I think it's fine because these use cases don't make much sense and users of this framework would be also confused about what the 100 % actually is.

The containerRelativeFrame is cool but unfortunately supported just by iOS 17+, as you mentioned.

GeometryReader could be utilized, so it also supports older iOS.
Here is an example of the relative sizes:
image.

Copy link
Author

Choose a reason for hiding this comment

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

I've submitted a new commit using GeometryReader to handle relative width size.

@paul1893 paul1893 force-pushed the feature/support-image-resizing-suffix branch from 336f136 to 47c147a Compare January 11, 2025 22:33
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