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

Add string interpolatability to InlineHTML. #277

Merged
merged 9 commits into from
Jan 23, 2025

Conversation

JPToroDev
Copy link
Collaborator

String Interpolation Support for InlineHTML

This PR introduces string interpolation capabilities for InlineHTML, similar to SwiftUI's Text view. This enhancement allows for more expressive and dynamic HTML content creation within string literals.

Example Usage

struct Important: InlineHTML {
   init(_ text: String) {
       self.text = text
   }
   var text: String
   var body: some HTML {
       Span(text)
           .foregroundStyle(.red)
           .fontWeight(.bold)
   }
}

You can now use this custom InlineHTML element directly within strings:

Text(markdown: #"""
Swift protocols define a blueprint of methods, properties,
and other requirements that suit a particular task or functionality.
\#(Important("Note:")) The protocol can then be adopted by a class, structure,
or enumeration to provide an actual implementation of those requirements.
"""#)

Implementation Details

The changes to make this work are actually very small. The reason so many files are touched is because:

func render(context: PublishingContext) -> String

Has become:

func render(context: PublishingContext?) -> String

Most HTML elements don't utilize the context during rendering. Making the context optional enables string interpolation for inline elements with minimal changes to the current architecture.

JPToroDev added 4 commits January 18, 2025 10:52
#Conflicts:
#	Sources/Ignite/Elements/CodeBlock.swift
#	Sources/Ignite/Elements/Dropdown.swift
#	Sources/Ignite/Elements/HTMLBody.swift
@JPToroDev
Copy link
Collaborator Author

I'm going to resubmit with a better approach.

@JPToroDev JPToroDev closed this Jan 22, 2025
@twostraws
Copy link
Owner

Okay. I did read this through and didn't feel comfortable about the publishing context being optional when really it's always present, but it made me wonder whether the publishing context is really some sort of singleton – it would allow us to remove it from the render() method entirely.

@JPToroDev
Copy link
Collaborator Author

I was wondering that too while putting this together. I honestly think that is the better implementation.

The alternative is making context required again, and creating an empty PublishingContext to feed into InlineHTML's description property.

But every property of PublishingContext used in render() is a static piece of data that wouldn't change within a site.

I'll reopen this PR with that approach for you to review.

@JPToroDev
Copy link
Collaborator Author

Meaner and leaner.

@JPToroDev JPToroDev reopened this Jan 22, 2025
@@ -1,35 +0,0 @@
//
Copy link
Owner

Choose a reason for hiding this comment

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

How come the subsite tests went away?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can re-add them, but I think that might be best in a separate PR. I think the subset tests need to be rethought a little. All but one of the subsite tests were testing elements that weren't tested for the regular site, so I moved them over there.

Copy link
Owner

Choose a reason for hiding this comment

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

Okay. Rather than re-add them, I wonder whether they could use Swift Testing's parameterized test system?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Excellent idea. I think that should be possible. I'll put something together in a small follow-up PR. 🙌🏼

@twostraws
Copy link
Owner

This has a couple of conflicts, but otherwise I think it's good to merge.

JP Toro added 2 commits January 23, 2025 10:57
#Conflicts:
#	Sources/Ignite/Elements/Slide.swift
#	Tests/IgniteTesting/Elements/Attributes.swift
@JPToroDev
Copy link
Collaborator Author

All set.

@twostraws
Copy link
Owner

Thank you!

@twostraws twostraws merged commit c2b6830 into twostraws:main Jan 23, 2025
1 check passed
@JPToroDev JPToroDev deleted the 011724d branch January 25, 2025 16:42
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