-
Notifications
You must be signed in to change notification settings - Fork 523
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
Allow setting thresholds
#485
base: main
Are you sure you want to change the base?
Conversation
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.
Might be nice to split this. Setting rootMargin
should mostly be uncontroversial. Setting thresholds might require a bit more thought.
index.bs
Outdated
readonly attribute DOMString rootMargin; | ||
readonly attribute FrozenArray<double> thresholds; | ||
attribute DOMString rootMargin; | ||
attribute FrozenArray<double> thresholds; |
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 don't think non-readonly FrozenArray
s are a thing?
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.
What should we use here?
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.
probably sequence<double>
? Though not a webIDL expert so...
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.
ObservableArray<double>
would be best, yeah. Attributes cannot be sequence<>
s. You can have settable FrozenArray<>
s, but the semantics are confusing, thus we invented ObservableArray<>
...
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.
attribute FrozenArray<double> thresholds; | |
attribute ObservableArray<double> thresholds; |
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.
Thanks, fixed.
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.
ObservableArrays don't have on-getting/on-setting algorithms though. They need a different setup, "set an indexed value" and "delete an indexed value". See https://webidl.spec.whatwg.org/#idl-observable-array
I can edit this branch to only change |
thresholds
Co-authored-by: Emilio Cobos Álvarez <emilio@crisal.io>
As with modifying rootMargin, we need to specify the effect of modifying thresholds on previousThresholdIndex: |
This thread was buried in my inbox and I got to it by doing some cleaning up. Out of curiosity, is there any progress here? @zcorpan Do you plan to continue this? |
@domfarolino if there's impl interest I can make a plan. :) |
I see. Maybe we can just ping @szager-chromium and @emilio, and I'm not sure for Apple? .... |
Part of #428.
The following tasks have been completed:
Implementation commitment:
Preview | Diff