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

Could the Html property on OEmbedResponse perhaps return an HtmlString #17

Open
soreng opened this issue Aug 5, 2021 · 3 comments
Open

Comments

@soreng
Copy link
Contributor

soreng commented Aug 5, 2021

I notice the use of Html.Raw ... could the Html property perhaps return an HtmlString instead,? That way the property can be rendered "straight up".

EDIT: this is not the place for such a comment. Sorry. My mistake! But now it's here - should it stay or should it go?

Originally posted by @jannikanker in #10 (comment)

@soreng
Copy link
Contributor Author

soreng commented Aug 5, 2021

It would need some testing if changed, and would also be a bit Breaking.

the html part of the response may / may not, be filled, so there needs to be checks to decide what to render.

It may be an idea to ship with a (partial) view that can render a non-styled version of the output, bit I would suggest having the documentation in place before changing code.

The OembedResponse is also the entity being returned by the OEmbed provider. Not quite sure how deserialising json to an object with an HtmlString-property.

@soreng soreng changed the title I notice the use of Html.Raw ... could the Html property perhaps return an HtmlString instead,? That way the property can be rendered "straight up". Could the Html property on OEmbedResponse perhaps return an HtmlString Aug 5, 2021
@bjarnef
Copy link

bjarnef commented Aug 7, 2021

Or maybe IHtmlString like many extension methods in Umbraco core?

@soreng
Copy link
Contributor Author

soreng commented Aug 8, 2021

Or maybe IHtmlString like many extension methods in Umbraco core?

IHtmlString is IHtmlContent in AspNetCore :)

I can see a few ways to make this work.

One way would be to create multiple versions of the OEmbedResponse. One for storing the response, and another version for rendering. The rendering version could then have a Html typed as IHtmlContent. In order to do this, the property value converter would then implement the mapping.

Another way, would be to "store" the IHtmlContent. This would then allow to only have a single version of the OEmbedResponse, but moving the "mapping" to the services that may / may not, know that they are used for anything AspNetCore related.

you would still need multiple types for this to work

Of the two, I would prefer the first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants