-
Notifications
You must be signed in to change notification settings - Fork 563
add HTML-helper DSL to FSharpKernel #405
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
Conversation
| abstract member HTML: obj -> FSharpPocketViewTags | ||
| abstract member Item: string * obj -> FSharpPocketViewTags with get | ||
| default __.HTML content = | ||
| p.SetContent([|content|]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a method ToHtmlContent that you can use to implement this without involving PocketView. PocketViewTags.HTML actually probably belongs with the methods under Microsoft.DotNet.Interactive.Kernel and the whole thing should probably move into Microsoft.DotNet.Interactive.Jupyter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I follow. If in C# I do div("foo") that will call TryInvoke which internally just calls SetContent() and the body of that method does all of the fancy HTML escaping and encoding.
The ToHtmlContent() methods that I found only operate on string, so I don't know how I would apply that in the F# case. E.g.,
let content = "foo".ToHtmlContent() // this is now of the type `IHtmlContent`
div.<what_do_I_put_here_so_that_I_can_pass_in_content_from_above>(content)This is made doubly difficult in that F# doesn't have dynamic and we're so strongly typed that it's sometimes painful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comment was only about the HTML method, which is unrelated to PocketView, and doesn't require dynamic.
public static IHtmlContent HTML(string content) => content.ToHtmlContent();There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give an example? Do you mean this?
-default __.HTML content =
- p.SetContent([|content|])
+default __.HTML (content: string) =
+ p.SetContent([|content.ToHtmlContent()|])There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm suggesting we remove HTML from PocketViewTags and move it to someplace where it can be called directly from F# submissions. It doesn't need an F# wrapper or alternate implementation. It belongs in the same class with display and Javascript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I think I see the confusion.
I'm simply trying to get around the fact that an F# object can't be directly invoked. E.g., this doesn't work:
div.["class", "c"]("body")
// ^^^^^^^^ this fails because
// ^^^^^^^^^^^^^^ this isn't a functionSo we have to have an actual function that can be invoked:
div.["class", "c"].HTML("body")
// ^^^^ this IS a function and it CAN be invokedOriginally I called this innerHTML to mimic the HTML DOM, but then I found HTML() on PocketViewTags and thought I'd mimic that. Ultimately I don't care what this is called; the only restriction is that it has to exist.
Is there a name you'd prefer, or is HTML (or innerHTML) fine?
Quick edit:
The display and Javascript methods you mentioned feel like the wrong place for this, at least for F#, because the API would look like this and it feels clunky:
a.["href", "bing.com"].HTML("Click for bing!") // this is the current state in this PR
content(a.["href", "bing.com"], "Click for bing!") // this feels terrible
a.["href", "bing.com"].content("Click for bing!") // having `content` be an extension method feels acceptable, see belowIf content() is an extension method then the API would have to look like this to be consumable by F#:
public static object content(this FSharpPocketViewTags tags, object body)
{
tags.somehow_we_get_access_to_the_underlying_PocketView_value.SetContent(body);
}And if we need an explicit overload just for F#, I'd rather leave it as it is in this PR; a dedicated method directly tied to the FSharpPocketViewTags class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spoke offline with @jonsequitur and @TIHan and long story short I mis-read the meaning of the HTML() method. To try to make the meaning clear I'm copying the DOM method name innerHTML().
There were other discussions about utilizing a constructor to set the inner content, but that opens a whole new can of worms around using the helper effectively.
This PR mimics the
PocketViewTagshelper items for F#. Since F# doesn't have dynamic invocation in the same way that C# does, I had to do things a little differently, but the end result is largely the same. The easiest approach is to simply show C#/F# equivalent pairs.C#:
F#
Result:
In action:
