Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] Avoid returning List from DOM apis. #33880

Merged
merged 1 commit into from
Jun 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 66 additions & 18 deletions lib/web_ui/lib/src/engine/dom.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ class DomDocument {}
extension DomDocumentExtension on DomDocument {
external DomElement? get documentElement;
external DomElement? querySelector(String selectors);
List<DomElement> querySelectorAll(String selectors) =>
js_util.callMethod<List<Object?>>(
this, 'querySelectorAll', <Object>[selectors]).cast<DomElement>();
Iterable<DomElement> querySelectorAll(String selectors) =>
_DomElementListWrapper.create(js_util.callMethod<_DomElementList>(
this, 'querySelectorAll', <Object>[selectors]));
DomElement createElement(String name, [Object? options]) =>
js_util.callMethod(this, 'createElement',
<Object>[name, if (options != null) options]) as DomElement;
Expand Down Expand Up @@ -172,8 +172,8 @@ class DomElement extends DomNode {}
DomElement createDomElement(String tag) => domDocument.createElement(tag);

extension DomElementExtension on DomElement {
List<DomElement> get children =>
js_util.getProperty<List<Object?>>(this, 'children').cast<DomElement>();
Iterable<DomElement> get children => _DomElementListWrapper.create(
js_util.getProperty<_DomElementList>(this, 'children'));
external int get clientHeight;
external int get clientWidth;
external String get id;
Expand All @@ -188,9 +188,9 @@ extension DomElementExtension on DomElement {
external DomRect getBoundingClientRect();
external void prepend(DomNode node);
external DomElement? querySelector(String selectors);
List<DomElement> querySelectorAll(String selectors) =>
js_util.callMethod<List<Object?>>(
this, 'querySelectorAll', <Object>[selectors]).cast<DomElement>();
Iterable<DomElement> querySelectorAll(String selectors) =>
_DomElementListWrapper.create(js_util.callMethod<_DomElementList>(
this, 'querySelectorAll', <Object>[selectors]));
external void remove();
external void setAttribute(String name, Object value);
void appendText(String text) => append(createDomText(text));
Expand All @@ -207,16 +207,14 @@ extension DomCSSStyleDeclarationExtension on DomCSSStyleDeclaration {
set clip(String value) => setProperty('clip', value);
set clipPath(String value) => setProperty('clip-path', value);
set transform(String value) => setProperty('transform', value);
set transformOrigin(String value) =>
setProperty('transform-origin', value);
set transformOrigin(String value) => setProperty('transform-origin', value);
set opacity(String value) => setProperty('opacity', value);
set color(String value) => setProperty('color', value);
set top(String value) => setProperty('top', value);
set left(String value) => setProperty('left', value);
set right(String value) => setProperty('right', value);
set bottom(String value) => setProperty('bottom', value);
set backgroundColor(String value) =>
setProperty('background-color', value);
set backgroundColor(String value) => setProperty('background-color', value);
set pointerEvents(String value) => setProperty('pointer-events', value);
set filter(String value) => setProperty('filter', value);
set zIndex(String value) => setProperty('z-index', value);
Expand Down Expand Up @@ -251,8 +249,7 @@ extension DomCSSStyleDeclarationExtension on DomCSSStyleDeclaration {
set borderRadius(String value) => setProperty('border-radius', value);
set perspective(String value) => setProperty('perspective', value);
set padding(String value) => setProperty('padding', value);
set backgroundImage(String value) =>
setProperty('background-image', value);
set backgroundImage(String value) => setProperty('background-image', value);
set border(String value) => setProperty('border', value);
set mixBlendMode(String value) => setProperty('mix-blend-mode', value);
set backgroundSize(String value) => setProperty('background-size', value);
Expand Down Expand Up @@ -655,11 +652,11 @@ extension DomHTMLTextAreaElementExtension on DomHTMLTextAreaElement {
class DomClipboard extends DomEventTarget {}

extension DomClipboardExtension on DomClipboard {
Future<String> readText() =>
js_util.promiseToFuture<String>(js_util.callMethod(this, 'readText', <Object>[]));
Future<String> readText() => js_util.promiseToFuture<String>(
js_util.callMethod(this, 'readText', <Object>[]));

Future<dynamic> writeText(String data) =>
js_util.promiseToFuture(js_util.callMethod(this, 'readText', <Object>[data]));
Future<dynamic> writeText(String data) => js_util
.promiseToFuture(js_util.callMethod(this, 'readText', <Object>[data]));
}

extension DomResponseExtension on DomResponse {
Expand Down Expand Up @@ -694,6 +691,57 @@ extension DomKeyboardEventExtension on DomKeyboardEvent {
external bool getModifierState(String keyArg);
}

/// [_DomElementList] is the shared interface for APIs that return either
/// `NodeList` or `HTMLCollection`. Do *not* add any API to this class that
/// isn't support by both JS objects. Furthermore, this is an internal class and
/// should only be returned as a wrapped object to Dart.
@JS()
@staticInterop
class _DomElementList {}

extension DomElementListExtension on _DomElementList {
external int get length;
DomElement item(int index) =>
js_util.callMethod<DomElement>(this, 'item', <Object>[index]);
}

class _DomElementListIterator extends Iterator<DomElement> {
final _DomElementList elementList;
int index = -1;

_DomElementListIterator(this.elementList);

@override
bool moveNext() {
index++;
if (index > elementList.length) {
throw 'Iterator out of bounds';
}
return index < elementList.length;
}

@override
DomElement get current => elementList.item(index);
}

class _DomElementListWrapper extends Iterable<DomElement> {
final _DomElementList elementList;

_DomElementListWrapper._(this.elementList);

/// This is a work around for a `TypeError` which can be triggered by calling
/// `toList` on the `Iterable`.
static Iterable<DomElement> create(_DomElementList elementList) =>
_DomElementListWrapper._(elementList).cast<DomElement>();

@override
Iterator<DomElement> get iterator => _DomElementListIterator(elementList);

/// Override the length to avoid iterating through the whole collection.
@override
int get length => elementList.length;
}

Object? domGetConstructor(String constructorName) =>
js_util.getProperty(domWindow, constructorName);

Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/html/surface_stats.dart
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ void debugPrintSurfaceStats(PersistedScene scene, int frameNumber) {
// A microtask will fire after the DOM is flushed, letting us probe into
// actual <canvas> tags.
scheduleMicrotask(() {
final List<DomElement> canvasElements = domDocument.querySelectorAll('canvas');
final Iterable<DomElement> canvasElements = domDocument.querySelectorAll('canvas');
final StringBuffer canvasInfo = StringBuffer();
final int pixelCount = canvasElements
.cast<DomCanvasElement>()
Expand Down
10 changes: 6 additions & 4 deletions lib/web_ui/test/engine/surface/scene_builder_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ void testMain() {

final DomElement contentAfterReuse = builder2.build().webOnlyRootElement!;
final List<DomCanvasElement> list =
contentAfterReuse.querySelectorAll('canvas').cast<DomCanvasElement>();
contentAfterReuse.querySelectorAll('canvas').cast<DomCanvasElement>().toList();
expect(list[0].style.zIndex, '-1');
expect(list[1].style.zIndex, '');
});
Expand All @@ -268,7 +268,7 @@ void testMain() {
final DomElement content = builder.build().webOnlyRootElement!;
domDocument.body!.append(content);
List<DomHTMLImageElement> list =
content.querySelectorAll('img').cast<DomHTMLImageElement>();
content.querySelectorAll('img').cast<DomHTMLImageElement>().toList();
for (final DomHTMLImageElement image in list) {
image.alt = 'marked';
}
Expand All @@ -284,7 +284,8 @@ void testMain() {
builder2.pop();

final DomElement contentAfterReuse = builder2.build().webOnlyRootElement!;
list = contentAfterReuse.querySelectorAll('img').cast<DomHTMLImageElement>();
list =
contentAfterReuse.querySelectorAll('img').cast<DomHTMLImageElement>().toList();
for (final DomHTMLImageElement image in list) {
expect(image.alt, 'marked');
}
Expand Down Expand Up @@ -515,7 +516,8 @@ void testMain() {
renderedLayers[char] = pushChild(builder, char, oldLayer: renderedLayers[char]);
}
final SurfaceScene scene = builder.build();
final List<DomElement> pTags = scene.webOnlyRootElement!.querySelectorAll('flt-paragraph');
final List<DomElement> pTags =
scene.webOnlyRootElement!.querySelectorAll('flt-paragraph').toList();
expect(pTags, hasLength(string.length));
expect(
scene.webOnlyRootElement!.querySelectorAll('flt-paragraph').map((DomElement p) => p.innerText).join(''),
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/test/text_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ Future<void> testMain() async {
paragraph.layout(const ParagraphConstraints(width: 800.0));
expect(paragraph.plainText, 'abcdef');
final List<Element> spans =
paragraph.toDomElement().querySelectorAll('flt-span').cast<Element>();
paragraph.toDomElement().querySelectorAll('flt-span').cast<Element>().toList();
expect(spans[0].style.fontFamily, 'Ahem, $fallback, sans-serif');
// The nested span here should not set it's family to default sans-serif.
expect(spans[1].style.fontFamily, 'Ahem, $fallback, sans-serif');
Expand Down