-
Notifications
You must be signed in to change notification settings - Fork 44
Mutable List interface for children and childNodes
#490
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
77c23f9
c3d91d8
b2c91fd
37605f0
8435092
17ba464
f9d9226
194b07e
8dcf595
c7bc010
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ import 'dart:convert'; | |
| import 'dart:js_interop'; | ||
|
|
||
| import '../dom.dart'; | ||
| import 'lists.dart'; | ||
|
|
||
| export 'cross_origin.dart' | ||
| show CrossOriginContentWindowExtension, CrossOriginWindowExtension; | ||
|
|
@@ -103,3 +104,14 @@ extension UriToURL on Uri { | |
| } | ||
| } | ||
| } | ||
|
|
||
| extension NodeExtension on Node { | ||
| /// Returns [childNodes] as a modifiable [List] | ||
| List<Node> get childNodesAsList => JSLiveNodeListWrapper(this, childNodes); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this should be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I cant see
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, sorry I meant |
||
| } | ||
|
|
||
| extension ElementExtension on Element { | ||
| /// Returns [children] as a modifiable [List] | ||
| List<Element> get childrenAsList => | ||
| JSLiveNodeListWrapper<Element, HTMLCollection, Element>(this, children); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| import 'dart:collection'; | ||
| import 'dart:js_interop'; | ||
| import '../dom/dom.dart'; | ||
|
|
||
| /// `_JSList` acts as a wrapper around a JS list object providing an interface to | ||
| /// access the list items and list length while also allowing us to specify the | ||
|
|
@@ -69,3 +70,44 @@ class JSImmutableListWrapper<T extends JSObject, U extends JSObject> | |
| @override | ||
| U elementAt(int index) => this[index]; | ||
| } | ||
|
|
||
| /// A wrapper for live node lists. `NodeList` and `HTMLCollection` that are | ||
| /// [live](https://developer.mozilla.org/en-US/docs/Web/API/NodeList#live_vs._static_nodelists) | ||
| /// can be safely modified at runtime. This requires an instance of `P`, a | ||
| /// container that elements would be added to or removed from. | ||
| class JSLiveNodeListWrapper<P extends Node, T extends JSObject, U extends Node> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think this is the same as
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this is realiable source: I've added |
||
| extends JSImmutableListWrapper<T, U> { | ||
| P parentNode; | ||
fsw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| JSLiveNodeListWrapper(this.parentNode, super.original); | ||
|
|
||
| @override | ||
| set length(int value) { | ||
| if (value > length) { | ||
| throw UnsupportedError('Cannot add null to live node List.'); | ||
| } | ||
| for (var i = length - 1; i >= value; i--) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this is new compared to the dart:html version. I think that's okay though since we were throwing in the dart:html version anyways.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, it is here to reuse JSImmutableListWrapper logic without overriding many methods |
||
| parentNode.removeChild(_jsList.item(i)); | ||
| } | ||
| } | ||
|
|
||
| @override | ||
| void operator []=(int index, U value) { | ||
| RangeError.checkValidRange(index, null, length); | ||
| parentNode.replaceChild(value, _jsList.item(index)); | ||
| } | ||
|
|
||
| @override | ||
| void add(U element) { | ||
| // `ListMixin` implementation only works for lists that allow `null`. | ||
| parentNode.appendChild(element); | ||
| } | ||
|
|
||
| @override | ||
| void removeRange(int start, int end) { | ||
| RangeError.checkValidRange(start, end, length); | ||
| for (var i = 0; i < end - start + 1; i++) { | ||
|
||
| parentNode.removeChild(this[start]); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be I think this would be less confusing if we just modified the given parameter e.g.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. d'oh, indeed I made But We can do something like: But not sure if this would be less confusing.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, the list is getting readjusted as you're removing elements. And yeah, I don't think the |
||
| } | ||
| } | ||
| } | ||
fsw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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.
nit: Dart doc comments should be full sentences.
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.
Hmm, I thought subjectless sentences are allowed? (sorry, I am not native speaker) or do you mean just the dot at the end?