Fix bugs across component, layout, navigator and async content

CHANGELOG.md

@@ -1,3 +1,87 @@
+## 3.0.19
+
+- `UIComponent`:
+  - `onChildRendered`: fixed the notification passing the parent component instead of the
+    rendered child (`_callOnChildRendered` called `onChildRendered(this)`).
+  - `_parseOnEventKeyPress`: an `onEventKeyPress` attribute without a `key:action` delimiter
+    threw a `RangeError` that aborted the whole render (`toContentElements` returned an empty
+    list). The value is now handled as an action triggered by any key.
+  - `focusField`: was looking for the text input in the component itself instead of in the
+    resolved field component.
+  - `getFieldsComponentsMap`: for a repeated field name it now keeps the `UIField` component
+    instead of letting a plain element overwrite it. `getFieldsExtended` had a sort with the
+    same intent, but it compared `MapEntry is UIComponent` (always `false`) over an
+    already deduplicated map.
+  - `appendStyle`: ensures the previous declarations are terminated before appending.
+
+- `UIAsyncContent`:
+  - `_ensureElementForDOM`: an HTML content with multiple root nodes returned the raw
+    `String` instead of the built element (missing `return` in the branch).
+
+- `UILayout`:
+  - `_getElementIndexByID`: an element without an `id` built the invalid CSS selector `#`,
+    throwing a `SyntaxError`. This broke *any* evaluated `uiLayout` command
+    (e.g. `x(10px)`) on elements without an `id`.
+  - `_parseValuePx`: crashed with a `FormatException` on fractional values (e.g. `4.5px`);
+    now parsed as a `num` and ignored when not a plain `px` value (e.g. `calc(...)`).
+  - `_valueXY`: returned the invalid CSS value `'0 px'` (with a space) when the parent had
+    no dimensions.
+  - The window `resize` listener is now registered once per `Window` (it was registered by
+    every `UILayout` instance, since `_registeredWindow` was not `static`).
+  - `_configure`: removed a no-op `split()` over a regex passed as a literal `String`.
+  - `_getElementIndex`/`_getElementIndexByID`: null-safe for detached elements.
+
+- `UINavigator`:
+  - `navigateOnClick`: a `null`/empty route threw a null-check error; it now clears a
+    previously registered route and returns `null`.
+  - `_navigateTo`: the "navigable not found" retry dropped the `fromURL` flag, pushing a
+    duplicated entry in the browser history for URL-driven navigations.
+
+- `UIElementExtension.resolveUIComponent`: no longer throws when there is no `UIRoot`.
+
+- `$uiButtonLoader`:
+  - `button-style` was only emitted when `buttonClasses` was provided (and was emitted empty
+    when only `buttonClasses` was given).
+  - `loaded-text-classes` was built from `buttonClasses` instead of `loadedTextClass`.
+  - `UIButtonLoader.generator` now also accepts the `loaded-text-classes` attribute
+    (the one generated by `$uiButtonLoader`).
+
+- `UIEventHandler.trackAllRegisteredEventListeners`: fixed an empty-check using `isElement`.
+
+- `getUILoadingType`: the `dualRing` case was unreachable, since the value is lower-cased
+  before the `switch`. `UILoadingType.dualRing` could never be parsed from a `String` and
+  silently fell back to `UILoadingType.ring`. Also accepts `dual-ring` and `dual_ring`.
+
+- `UILoadingConfig`:
+  - `toInlineProperties` emitted `textZoom`, but `fromMap` only read `text-zoom`, and
+    `withProgress` was not emitted at all. Both were silently lost on a
+    `toInlineProperties` → `parse` round-trip (used by the `ui-button-loader` tag).
+
+- `UIDialog.getAllDialogs`/`removeAllDialogs`: resolved the dialogs by mapping the
+  `.ui-dialog` elements back to their components through an `Expando`. An `Expando` is
+  keyed by identity, and on `dart2wasm` an element re-read from the DOM is a new Dart
+  wrapper, so the lookup always missed and both methods were no-ops. They now resolve
+  the instances through `UIRootComponent.getInstances`.
+
+- Lints (`dart analyze --fatal-infos --fatal-warnings`):
+  - `BUIViewProvider.fromManifestContent`: the `return` of the `async`
+    `fromManifestTree` was inside a `try` block, so its errors were never caught by the
+    `catch` (`unawaited_return_in_try_block`). Moved out of the `try`, which keeps the
+    behavior and makes the guarded scope explicit.
+  - Converted the forwarded constructor parameters to super parameters in `UIDocument`,
+    `BUIRender`, `UIButtonCapture`, `UIDialogInput`, `UIDialogAlert` and `UIDialogLoading`
+    (`use_super_parameters`).
+
+- Tests:
+  - Added `test/bones_ui_component_test.dart`, `test/bones_ui_async_content_test.dart`,
+    `test/bones_ui_layout_test.dart`, `test/bones_ui_navigator_test.dart` and
+    `test/bones_ui_button_test.dart`.
+  - Coverage: added `test/bones_ui_extension_test.dart`,
+    `test/bones_ui_component_async_test.dart`, `test/bones_ui_fields_test.dart`,
+    `test/bones_ui_navigable_test.dart`, `test/bones_ui_utils_test.dart` and
+    `test/bones_ui_dialog_test.dart`.
+  - Line coverage: 47.8% → 60.8%.
+
 ## 3.0.18
 
 - `UIDocument`:

lib/src/bones_ui.dart

@@ -1,3 +1,3 @@
 class BonesUI {
-  static const String version = '3.0.18';
+  static const String version = '3.0.19';
 }

lib/src/bones_ui_async_content.dart

@@ -390,7 +390,7 @@ Object? _ensureElementForDOM(Object? element) {
       if (childNodes.length == 1) {
         return childNodes.toIterable().first;
       } else {
-        div;
+        return div;
       }
     } else {
       var span = HTMLSpanElement();

lib/src/bones_ui_base.dart

@@ -104,7 +104,7 @@ abstract class _EventHandlerPrivate {
   void _trackAllRegisteredEventListeners(
     List<RegisteredEventListener>? listeners,
   ) {
-    if (listeners == null || listeners.isElement) return;
+    if (listeners == null || listeners.isEmpty) return;
 
     var registeredEventListeners = _registeredEventListeners ??= [];
     registeredEventListeners.addAll(listeners);

lib/src/bones_ui_component.dart

@@ -830,11 +830,16 @@ abstract class UIComponent extends UIEventHandler {
 
       final content = this.content!;
 
-      var cssText = content.style.cssText;
-      if (cssText == '') {
+      var cssText = content.style.cssText.trim();
+      if (cssText.isEmpty) {
         cssText = allStylesLine;
       } else {
-        cssText += allStylesLine;
+        // Ensure that the previous declarations are terminated, otherwise
+        // the appended entry would be merged into the last property value:
+        if (!cssText.endsWith(';')) {
+          cssText += ';';
+        }
+        cssText += ' $allStylesLine';
       }
 
       content.style.cssText = cssText;
@@ -1049,9 +1054,19 @@ abstract class UIComponent extends UIEventHandler {
     List<String>? fields,
     List<String>? ignoreFields,
   }) {
-    var map = Map<String, Object>.fromEntries(
-      _listFieldsEntriesInContentDeepImpl(_content!.children.toList()),
-    );
+    var map = <String, Object>{};
+
+    for (var entry in _listFieldsEntriesInContentDeepImpl(
+      _content!.children.toList(),
+    )) {
+      var value = entry.value;
+
+      // For a repeated field name prefer an `UIComponent` over a plain
+      // element, since it resolves the field value through `UIField`:
+      if (map[entry.key] is UIComponent && value is! UIComponent) continue;
+
+      map[entry.key] = value;
+    }
 
     if (fields != null) {
       map.removeWhere((key, value) => !fields.contains(key));
@@ -1982,7 +1997,7 @@ abstract class UIComponent extends UIEventHandler {
 
   void _callOnChildRendered(UIComponent child) {
     try {
-      onChildRendered(this);
+      onChildRendered(child);
     } catch (e, s) {
       logger.error('Error calling onChildRendered() for instance: $this', e, s);
     }
@@ -3306,34 +3321,11 @@ abstract class UIComponent extends UIEventHandler {
     List<String>? fields,
     List<String>? ignoreFields,
   }) {
-    var fieldsElementsMap = getFieldsComponentsMap(
-      fields: fields,
-      ignoreFields: ignoreFields,
+    // Repeated field names are already resolved by [getFieldsComponentsMap],
+    // that prefers an `UIComponent` over a plain element:
+    return Map<String, Object?>.of(
+      getFieldsComponentsMap(fields: fields, ignoreFields: ignoreFields),
     );
-
-    var entries = fieldsElementsMap.entries.toList();
-    entries.sort((a, b) {
-      var aIsUIComponent = a is UIComponent;
-      var bIsUIComponent = b is UIComponent;
-
-      if (aIsUIComponent && !bIsUIComponent) {
-        return -1;
-      } else if (bIsUIComponent && !aIsUIComponent) {
-        return 1;
-      } else {
-        return 0;
-      }
-    });
-
-    var fieldsValues = <String, Object?>{};
-
-    for (var entry in entries) {
-      var key = entry.key;
-      if (fieldsValues.containsKey(key)) continue;
-      fieldsValues[key] = entry.value;
-    }
-
-    return fieldsValues;
   }
 
   Map<String, dynamic>? _renderedFieldsValues;
@@ -3622,7 +3614,7 @@ abstract class UIComponent extends UIEventHandler {
       (component as HTMLElement).focus();
       return true;
     } else if (component is UIComponent) {
-      var input = findInContentChildDeep((e) => e.isTextInput);
+      var input = component.findInContentChildDeep((e) => e.isTextInput);
       if (input != null) {
         return input.focus();
       }
@@ -3725,9 +3717,14 @@ abstract class UIComponent extends UIEventHandler {
     var keypress = getElementAttribute(elem, 'onEventKeyPress');
 
     if (keypress != null && keypress.isNotEmpty) {
-      var parts = keypress.split(':');
-      var key = parts[0].trim();
-      var actionType = parts[1];
+      var idx = keypress.indexOf(':');
+
+      // Without a `key:action` delimiter the whole value is the action,
+      // triggered by any key:
+      var key = idx >= 0 ? keypress.substring(0, idx).trim() : '*';
+      var actionType = idx >= 0 ? keypress.substring(idx + 1) : keypress;
+
+      if (key.isEmpty) key = '*';
 
       if (key == '*') {
         addTrackedEventListener(

lib/src/bones_ui_document.dart

@@ -139,11 +139,11 @@ class UIDocument extends UIComponentAsync {
     mk.ExtensionSet? markdownExtensionSet,
     loadingContent,
     errorContent,
-    dynamic classes,
-    dynamic classes2,
-    dynamic style,
-    dynamic style2,
-    dynamic id,
+    super.classes,
+    super.classes2,
+    super.style,
+    super.style2,
+    super.id,
   }) : _markdownExtensionSet = markdownExtensionSet,
        _resourceContent = resourceContent,
        super(
@@ -153,11 +153,6 @@ class UIDocument extends UIComponentAsync {
          loadingContent,
          errorContent,
          componentClass: 'ui-document',
-         classes: classes,
-         classes2: classes2,
-         style: style,
-         style2: style2,
-         id: id,
          generator: generator,
        );
 

lib/src/bones_ui_extension.dart

@@ -15,7 +15,7 @@ extension UIElementExtension on UIElement {
     if (parentUIComponent != null) {
       return parentUIComponent.findUIComponentByChild(this);
     } else {
-      return UIRoot.getInstance()!.findUIComponentByChild(this);
+      return UIRoot.getInstance()?.findUIComponentByChild(this);
     }
   }
 

lib/src/bones_ui_layout.dart

@@ -612,18 +612,25 @@ class UILayout {
   }
 
   int _getElementIndex(UIElement elem) {
-    var idx = elem.parentElement!.children.indexOf(elem);
-    return idx;
+    var parent = elem.parentElement;
+    if (parent == null) return -1;
+    return parent.children.indexOf(elem);
   }
 
   int _getElementIndexByID(UIElement elem) {
-    var elemID = elem.id;
-    List<UIElement> elemsSameID = elem.parentElement!
+    var elemID = elem.id.trim();
+    // An empty ID would build the invalid CSS selector `#`:
+    if (elemID.isEmpty) return -1;
+
+    var parent = elem.parentElement;
+    if (parent == null) return -1;
+
+    List<UIElement> elemsSameID = parent
         .querySelectorAll('#$elemID')
         .toElements();
     if (elemsSameID.isEmpty) return -1;
-    var idx = elemsSameID.indexOf(elem);
-    return idx;
+
+    return elemsSameID.indexOf(elem);
   }
 
   Map<String, int> _getElementCenter(HTMLElement elem) {
@@ -638,8 +645,10 @@ class UILayout {
     return {'x': x2, 'y': y2};
   }
 
-  Window? _registeredWindow;
+  static Window? _registeredWindow;
 
+  /// Registers the `resize` listener only once per [Window], since
+  /// [_onWindowResize] refreshes all the [UILayout] instances.
   void _registerWindowResize() {
     if (_registeredWindow != window) {
       _registeredWindow = window;
@@ -726,8 +735,7 @@ class UILayout {
 
     element.style.position = 'absolute';
 
-    var commands = this.layout.split('\\s*;\\s*');
-    commands.forEach(_commandsParser);
+    _commandsParser(layout);
   }
 
   void _commandsParser(String cmds) {
@@ -850,7 +858,7 @@ class UILayout {
 
       if (pw == 0 || ph == 0) {
         _needRefresh = true;
-        return '0 px';
+        return '0px';
       }
 
       return valueCenter(pw, ph, w, h);
@@ -940,10 +948,12 @@ class UILayout {
     return context;
   }
 
-  int? _parseValuePx(String val) {
+  /// Parses a `px` value, returning `null` if [val] is not a plain
+  /// `px` value (like `calc(...)` or a value with another unit).
+  num? _parseValuePx(String val) {
+    val = val.trim();
     if (val.endsWith('px')) {
-      var n = int.parse(val.substring(0, val.length - 2));
-      return n;
+      return num.tryParse(val.substring(0, val.length - 2).trim());
     }
     return null;
   }

lib/src/bones_ui_navigator.dart

@@ -418,6 +418,7 @@ class UINavigator {
           route,
           parameters: parameters,
           force: force,
+          fromURL: fromURL,
           cantFindNavigableRetry: cantFindNavigableRetry + 1,
         ),
       );
@@ -613,13 +614,20 @@ class UINavigator {
     ParametersProvider? parametersProvider,
     bool force = false,
   ]) {
+    if (route == null || route.isEmpty) {
+      if (element.isA<HTMLElement>()) {
+        clearNavigateOnClick(element as HTMLElement);
+      }
+      return null;
+    }
+
     var paramsStr = encodeQueryString(parameters);
 
     var attrRoute = element.getAttribute('__navigate__route');
     var attrParams = element.getAttribute('__navigate__parameters');
 
     if (route != attrRoute || paramsStr != attrParams) {
-      element.setAttribute('__navigate__route', route!);
+      element.setAttribute('__navigate__route', route);
       element.setAttribute('__navigate__parameters', paramsStr);
 
       var subscriptionHolder = <StreamSubscription>[];