Right now amp-list
is limited to handling responses that return an array somewhere in the json, it then iterates the array populating a template for each element.
Would it be possible to have it also accept non array values? In the case where the response was not an array it would treat it as a single entry array and populate the template once. This might be as simple as when the target is not an array just wrapping it in one rather than throwing an error.
This would greatly increase the use cases for amp-list with existing API endpoints.
@jpettitt sounds good to me.
after this change, it might be a good time to introduce an alias for amp-list
to make this more semantic. Maybe:
amp-template-renderer
?amp-mustache-renderer
?amp-partial
?amp-remote-renderer
?(this will be purely an alias, both amp-list and this will behave exactly the same. In documentation we will target them for different use-cases)
@cramforce opinions on naming?
/cc @ericlindley-g
We cannot rename amp-list
or I would lose me example for the worst named AMP extension :)
I'm all for this. amp-partial
is great. I'm also happy to make that an alias and retrofit amp-list
to support the non-list case.
I'm going to fix this one. I have an implementation question:
Should I silently convert non array values to array or should I add a flag to trigger the behavior?
Silently converting has the potential to be a breaking change ...
How do you want to disambiguate object-with-items-attribute as opposed to normal object?
In the silent fix scenario if the items
elements exists and it's not an array wrap it in one.
In the explicit flag scenario add an attribute single-result
that tells amp-list
that whatever is specified by the items
attribute is not an array (and internally wrap it in one)
I'm leaning towards the second option the more I think about it. I'm not attached to the flag name - suggestions?
Me, too. We could name the attribute not-actually-a-list
:)
Or rake the plunge and make amp-render
amp-render
seems like overkill and also invokes a risk of bike shedding what should be a very minor change. I kinda like not-actually-a-list
- I'll use that unless I hear otherwise.
Agree with attribute, but let's not call it 'not-actually-a-list' :) single-result
sgtm.
Any objection to adding a max-length
attribute too? This would limit the number of list items rendered in the case where a much longer list is returned. Again the issue here is making it easier to support existing endpoints. The specific use case I have in mind is limiting to 1 item but it's easy enough to make int arbitrary. It's 4 lines of code.
sgtm, maybe max-items
?
@aghassemi I used max-length in the pull request - want me to change it?
given items
is used throughout this component, max-items
would be more consistent. Would appreciate if you could change it in the PR.
Change made.
This issue is a canonical example of the first item on this list.
EDIT: It looks like the amp-list ampbyexample doc does not contain the changes in this commit yet. Maybe I am trying to use a feature which is not yet released?
EDIT2: Ah, ok. According to Releases, "Add support for single-result and max-length fixes #9687 (#11236)" is currently in Pre-release 1505349390841, but not yet in a _Latest release_. Sorry for the noise, I look forward to the feature in a week or two.
Does single-result
work as expected in conjunction with items="."
? Given the following object:
{
"title":"mytitle",
"iterable": [
{
"item1":"thing1",
"item2":"thing2"
}
]
}
I expected <amp-list single-origin items="." ...>
to process the following:
[{
"title":"mytitle",
"iterable": [
{
"item1":"thing1",
"item2":"thing2"
}
]
}]
Am I interpreting the combination of these attributes wrong?
@mattwomple will be in prod next Wednesday
Hi all,
quick check on the status and documentation of this change, is it all ready?
@AndrewKGuan , yes. See single-item
attribute on the documentation: https://www.ampproject.org/docs/reference/components/amp-list
Thanks, Ali!
Most helpful comment
Change made.
This issue is a canonical example of the first item on this list.