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

Schema parameter name is not a good name #13

Open
tomlm opened this issue Jan 30, 2020 · 1 comment
Open

Schema parameter name is not a good name #13

tomlm opened this issue Jan 30, 2020 · 1 comment

Comments

@tomlm
Copy link

tomlm commented Jan 30, 2020

I am one of the creators of Adaptive Cards at Microsoft and I love this component and project, great job!

My only nit is that the parameter name "Schema" is not a good name. A Schema is a description of the SHAPE of a json payload, not the json payload itself.

A much better name would be Card, or to match other blazor components Content

<AdaptiveCard Card="@card"></AdaptiveCard>

Even better would be to have the card be the content of the tag, like this:

<AdaptiveCard>
 @childContent
</AdaptiveCard>

This would allow templating to be injected into the definition of the json.

@mikoskinen
Copy link
Owner

Hi @tomlm and thanks for the comment and for the suggestions!

The child content implementation was something that was planned at some point but I'm not quite sure why I ended up using the "Schema"-property during the implementation. But as you mention, defining the card's content through child content would make sense.

I think the following two changes could be implemented based on your feedback:

  • AdaptiveCard.Schema -property is marked as obsolete.
  • Card content can be set using the Blazor's ChildContent support.

Regarding the other two components included in the library, meaning AdaptiveCards and TemplatedAdaptiveCard, using the "Schema" as the property name makes somewhat sense, because the idea is that you have the card's schema (or should it be template?) and the model which together form the actual card content. What to do you think?

What is actually completely missing from the AdaptiveCards-component is the ability to directly show a collection of cards without having to define the models and the schema. Given a scenario (which I just last week bumped into at work) where a backend returns multiple cards and we want to render them. It's not possible to do that easily using AdaptiveCards-component, because it always requires a single schema which is used to render all the models.

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

No branches or pull requests

2 participants