-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
amp-live-list: support amp-bind #7557
Comments
/to @choumx, @erwinmombay |
/to @kmh287 |
This seems useful, but I want to make sure I understand the value of doing this: @sebastianbenz — Since developers can specify updates to children of amp-live-list, what new use-cases would be supported by (1)? Re: (2), is it possible to filter content now, or will this ability only be useful in the future when it is possible to filter? |
/to @sebastianbenz For Eric's questions above. |
One use case: You can use amp-live-list to automatically update content on a page (by updating an item's timestamp). Together with a focus action (see #7626) it'd be possible to bring the updated element into the viewport, e.g. showing the last message in a message stream. |
Nice — sounds good to me :) Also I could see this being used to create a custom interaction to update content, rather than strictly using the existing update button (if we also exposed the trigger to slot the new items into the page) |
I'm a bit worried about people abusing this to run amp-bind on page load. |
Agree with Malte about (1) since We can start with (2) which is being implemented in #7990. |
Yikes — definitely agree with the risk here, and with the solution of starting with #7990 |
I don't understand why this would enable running amp-bind on page load. I'd expect this event to only fire the first time an amp-live-list is updated, which is min 15s after page load. |
Could someone abuse 1 to periodically refresh bind attributes on an entire page? |
Trying to figure out whether running this 15s after page load is better or worse :) Could this be limited to only if live list found a delta? In that case it would be fine, I think. |
Reopening to track (1) in OP. |
Sorry, my bad. |
Haven't checked in on this in a bit, but I'm assuming the event never fires without explicit user interaction (user taps the update button), as opposed to triggering when new data comes in. Is that the case? |
@ericlindley-g Current behavior post #7990 is that it does fire without user interaction, but only if there is an update. This admittedly does make me a bit nervous. |
Got it — I feel pretty strongly that we should only refresh bind attributes on explicit user action, unless we impose strict guidelines on what can be affected by these changes (no ads, no forms, no iframes, etc... — which I'm guessing isn't really feasible). WDYT? Explicit user action would be tapping on a button |
SGTM. Would you mind filing an issue and cc'ing me and @kmh287? |
Since we won't be going forward with 1, I'll close this issue. |
@ericlindley-g what are you concerns regarding automatically refreshing bind attributes with amp-live-list? What is this going to enable what's not already possible with amp-live-list? |
If I understand correctly, there are a few potential ways for
(2) & (3) don't pose any risk as far as I can see: (1) is where risk comes in. I think there are a number of abuse cases, but the one that comes to mind immediately is loading an interstitial ad X seconds after page load, by just changing the CSS styling on a div that contains an iframe or Though you also make a good point — maybe something similar is already possible with amp-live-list, even if it doesn't use amp-bind (like showing an interstitial ad * without * a close affordance). It's worth thinking more deeply about the core AMP principles, how we handle risks in relation to them, and how tradeoffs are evaluated. |
I would like to punt on #2 and #1 unless there is very strong demand on it.
I'd much rather make this decision in 6 months when we have learned a bit
more about amp-bind in the wild.
…On Mon, Apr 3, 2017 at 5:18 PM, ericlindley-g ***@***.***> wrote:
If I understand correctly, there are a few potential ways for
amp-live-list updates to support amp-bind:
1.
Trigger bind events when new data comes in (rather than on user
interaction with update button, which is already supported)
2.
Trigger updates to amp-state data when new amp-live-list updates come
in
3.
Ensure that bindable elements inside of new amp-live-list items are
properly bound, since they come in after page load
(2) & (3) don't pose any risk as far as I can see:
We definitely want to support (3), unless it's already supported by
default ( @choumx <https://github.com/choumx> , @kmh287
<https://github.com/kmh287> — is that the case, or do we need to build in
support for this?).
(2) feels nice to have (though I'd be curious to hear what others think —
if we're fetching data for amp-state at load time to better support
e-comm, it might make sense to also enable this kind of polling for the
freshest data after the initial load).
(1) is where risk comes in. I think there are a number of abuse cases, but
the one that comes to mind immediately is loading an interstitial ad X
seconds after page load, by just changing the CSS styling on a div that
contains an iframe or amp-ad. It would take kind of a weird
implementation on the backend to do this (publishing a page that every so
often has a full-page ad), but there's still some amount of risk.
Though you also make a good point — maybe something similar is already
possible with amp-live-list, even if it doesn't use amp-bind (like showing
an interstitial ad * without * a close affordance). It's worth thinking
more deeply about the core AMP principles, how we handle risks in relation
to them, and how tradeoffs are evaluated.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7557 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAFeTxIL94lItSdg3w1roURIrUWteLI8ks5rsYxfgaJpZM4MA-lx>
.
|
@cramforce — SGTM :) @choumx & @kmh287 — just to make sure, does amp-bind support #3 already, or do we need to build in additional support? |
@ericlindley-g Sorry I just saw this. 😅 |
Perfect—thanks @kmh287 ! |
I'd be great if amp-live-list would support amp-bind. Two scenarios:
<amp-live-list on="update:setState(liveBlogUpdated=true)" ...>
). This be good to be able to update the elements in reaction to an update.//cc @choumx
The text was updated successfully, but these errors were encountered: