From 0c1c97ad8a65f27e1a0ea4bed6741f9340ca9692 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 23:57:54 +0000 Subject: [PATCH] Load a layout's variables and conditions even when they arrive after it A downloaded layout was added to the publish model before its variables and conditions finished downloading. Any rebuild in that window (another layout emitting the model, the status stream) built the layout without them, and CodelesslyLayoutBuilder only reloads when the layout itself changes, so the layout stayed without them: conditions never applied and set variable actions found no variable. A layout cached without them had the same result. The data manager now adds a downloaded layout only after its variables and conditions, and the layout builder reloads when they change. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01B86QWEkjWBfHUrfXKRv6i5 --- lib/src/data/data_manager.dart | 12 +- lib/src/ui/layout_builder.dart | 25 +- test/fixtures/option_variance.json | 363 +++++++++++++++++++++++++++++ test/layout_loading_test.dart | 209 +++++++++++++++++ 4 files changed, 605 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/option_variance.json create mode 100644 test/layout_loading_test.dart diff --git a/lib/src/data/data_manager.dart b/lib/src/data/data_manager.dart index 3c6b99ff..2f381d4f 100644 --- a/lib/src/data/data_manager.dart +++ b/lib/src/data/data_manager.dart @@ -1418,7 +1418,8 @@ class DataManager { // Process Layouts SDKPublishLayout? layout; - if (model!.layouts.containsKey(layoutID)) { + final bool isCached = model!.layouts.containsKey(layoutID); + if (isCached) { DebugLogger.instance.printInfo( '\tLayout [$layoutID] is already cached. Skipping download.', name: name, @@ -1446,8 +1447,6 @@ class DataManager { name: name, ); } - - model.layouts[layoutID] = layout; } assert(layout != null, 'Layout should not be null at this point.'); @@ -1545,6 +1544,13 @@ class DataManager { ); } + // Add a downloaded layout only once its variables and conditions are in + // the model. A CodelesslyWidget builds a layout as soon as the model has + // it, and only loads the layout's variables and conditions then, so a + // layout added first would render without them if anything rebuilt the + // widget while they were still downloading. + if (!isCached) model.layouts[layoutID] = layout!; + await emitPublishModel(); savePublishModel(); diff --git a/lib/src/ui/layout_builder.dart b/lib/src/ui/layout_builder.dart index ffa6ee67..9e509e5b 100644 --- a/lib/src/ui/layout_builder.dart +++ b/lib/src/ui/layout_builder.dart @@ -156,16 +156,37 @@ class _CodelesslyLayoutBuilderState extends State { } } + /// The layout's variables and conditions that [loadLayout] last loaded. + SDKLayoutVariables? loadedVariables; + SDKLayoutConditions? loadedConditions; + @override void didUpdateWidget(covariant CodelesslyLayoutBuilder oldWidget) { super.didUpdateWidget(oldWidget); if (oldWidget.layout.id != widget.layout.id || oldWidget.canvasId != widget.canvasId || - oldWidget.layout.lastUpdated != widget.layout.lastUpdated) { + oldWidget.layout.lastUpdated != widget.layout.lastUpdated || + didVariablesOrConditionsChange(context)) { loadLayout(context); } } + /// Whether the publish model has different variables or conditions for + /// this layout than [loadLayout] last loaded. They can arrive after the + /// layout, e.g. when the layout was cached without them, or be published + /// on their own. + bool didVariablesOrConditionsChange(BuildContext context) { + final SDKPublishModel? model = context + .read() + .dataManager + .publishModel; + if (model == null) return false; + bool changed(Object? loaded, Object? current) => + !identical(loaded, current) && loaded != current; + return changed(loadedVariables, model.variables[widget.layout.id]) || + changed(loadedConditions, model.conditions[widget.layout.id]); + } + /// Loads the layout into the [nodeRegistry] and updates the /// [CodelesslyContext] with the node values. void loadLayout(BuildContext context) { @@ -219,6 +240,8 @@ class _CodelesslyLayoutBuilderState extends State { } } final publishModel = context.read().dataManager.publishModel!; + loadedVariables = publishModel.variables[widget.layout.id]; + loadedConditions = publishModel.conditions[widget.layout.id]; final Map variablesMap = publishModel.variables[widget.layout.id]?.variables[widget.canvasId] ?? diff --git a/test/fixtures/option_variance.json b/test/fixtures/option_variance.json new file mode 100644 index 00000000..2e5d3d91 --- /dev/null +++ b/test/fixtures/option_variance.json @@ -0,0 +1,363 @@ +{ + "layout": { + "owner": "owner", + "users": [ + "owner" + ], + "roles": { + "owner": "owner" + }, + "teams": [], + "public": false, + "id": "layout", + "pageId": "lhsT30U4KeVevueTi2Dw", + "projectId": "project", + "canvases": { + "0RA4iOShqVSy59V7Bb2w": { + "0RA4p4dd_3sheXfLa2Gq": { + "id": "0RA4p4dd_3sheXfLa2Gq", + "name": "Title", + "horizontalFit": "expanded", + "verticalFit": "shrinkWrap", + "characters": "Designer", + "alignment": { + "data": [ + 0, + 0 + ] + }, + "textMixedProps": [ + { + "fills": [ + { + "id": "0RA4OxNCFDi1dhybrf9_", + "type": "solid", + "color": "#FFFFFF" + } + ], + "fontName": { + "family": "Poppins", + "weight": "w400" + }, + "start": 0, + "end": 8 + } + ], + "textAlignHorizontal": "center", + "basicBoxLocal": [ + 16, + 61.5, + 118, + 27 + ], + "type": "text" + }, + "0RA4iOSkIRuraEIQ3JCQ": { + "reactions": [ + { + "name": "Set Variable Action 0", + "action": { + "type": "setVariable", + "variable": { + "id": "MTIQQwtDf15bwJZN2iHD", + "name": "role", + "extra": {} + }, + "newValue": "Designer" + }, + "trigger": { + "type": "click" + }, + "id": "0RJ9jE7c55gyxYRON_uf" + }, + { + "name": "Set Variable Action 1", + "action": { + "type": "setVariable", + "variable": { + "id": "cDFrs37hn0cmI3XqWq20", + "name": "isOther", + "value": "false", + "type": "boolean", + "extra": {} + }, + "newValue": "false" + }, + "trigger": { + "type": "click" + }, + "id": "0RJ9jE7cJc4GAAixj__E" + } + ], + "id": "0RA4iOSkIRuraEIQ3JCQ", + "name": "Unselected State", + "aspectRatioLock": true, + "alignment": { + "data": [ + 0, + 0 + ] + }, + "basicBoxLocal": [ + 0, + 0, + 150, + 150 + ], + "padding": 16, + "children": [ + "0RA4iOSk6VU9aIAf0t5Z" + ], + "inkWell": { + "overlayColor": "#1E5C69E5" + }, + "fills": [ + { + "id": "0RA4Oo8RkfluFf6Iowyv", + "type": "solid", + "opacity": 0.106, + "color": "#9E9E9E" + } + ], + "cornerRadius": [ + 12, + false, + "circular" + ], + "type": "frame" + }, + "0RA4iOSkYO3SgMtPqrJq": { + "id": "0RA4iOSkYO3SgMtPqrJq", + "name": "Option", + "basicBoxLocal": [ + 0, + 0, + 150, + 150 + ], + "allowedTypes": [], + "deniedTypes": [], + "type": "variance", + "currentVariantId": "default", + "variants": [ + { + "name": "unselected", + "id": "default", + "children": [ + "0RA4iOSkIRuraEIQ3JCQ" + ] + }, + { + "name": "selected", + "id": "0RA4PZebIlc5dVbAOoAU", + "children": [ + "0RA4p4dcPLrhyyT2RYhQ" + ] + } + ], + "properties": {} + }, + "0RA4iOShqVSy59V7Bb2w": { + "id": "0RA4iOShqVSy59V7Bb2w", + "name": "Onboarding Questionnaire", + "verticalFit": "shrinkWrap", + "aspectRatioLock": true, + "basicBoxLocal": [ + 7622.659, + 9651.269, + 1024, + 724 + ], + "children": [ + "0RA4iOShsBBpHJZNdZKp" + ], + "fills": [], + "mainAxisAlignment": "start", + "crossAxisAlignment": "start", + "type": "canvas", + "createdTimestamp": 1698127870741, + "properties": { + "bodyId": "0RA4iOShsBBpHJZNdZKp" + } + }, + "0RA4iOSk6VU9aIAf0t5Z": { + "id": "0RA4iOSk6VU9aIAf0t5Z", + "name": "Title", + "horizontalFit": "expanded", + "verticalFit": "shrinkWrap", + "characters": "Designer", + "alignment": { + "data": [ + 0, + 0 + ] + }, + "textMixedProps": [ + { + "fills": [ + { + "id": "0RA4OxNCFDi1dhybrf9_", + "type": "solid", + "color": "#000000" + } + ], + "fontName": { + "family": "Poppins", + "weight": "w400" + }, + "start": 0, + "end": 8 + } + ], + "textAlignHorizontal": "center", + "basicBoxLocal": [ + 16, + 63, + 118, + 24 + ], + "type": "text" + }, + "0RA4p4dcPLrhyyT2RYhQ": { + "id": "0RA4p4dcPLrhyyT2RYhQ", + "name": "Selected State", + "aspectRatioLock": true, + "alignment": { + "data": [ + 0, + 0 + ] + }, + "basicBoxLocal": [ + 0, + 0, + 150, + 150 + ], + "padding": 16, + "children": [ + "0RA4p4dd_3sheXfLa2Gq" + ], + "inkWell": { + "overlayColor": "#1E5C69E5" + }, + "fills": [ + { + "id": "0RA4Oo8RkfluFf6Iowyv", + "type": "solid", + "color": "#5C69E5" + } + ], + "cornerRadius": [ + 12, + false, + "circular" + ], + "type": "frame" + }, + "0RA4iOShsBBpHJZNdZKp": { + "id": "0RA4iOShsBBpHJZNdZKp", + "name": "Placeholder (Freeform)", + "horizontalFit": "expanded", + "verticalFit": "shrinkWrap", + "basicBoxLocal": [ + 0, + 0, + 1024, + 724 + ], + "children": [ + "0RA4iOSkYO3SgMtPqrJq" + ], + "type": "freeformPlaceholder" + } + } + }, + "breakpoints": [], + "lastUpdated": 1707144239010, + "canvasIds": [ + "0RA4iOShqVSy59V7Bb2w" + ] + }, + "variables": { + "owner": "owner", + "users": [ + "owner" + ], + "roles": { + "owner": "owner" + }, + "teams": [], + "public": false, + "id": "layout", + "variables": { + "0RA4iOShqVSy59V7Bb2w": { + "MTIQQwtDf15bwJZN2iHD": { + "id": "MTIQQwtDf15bwJZN2iHD", + "name": "role", + "extra": {} + } + } + } + }, + "conditions": { + "owner": "owner", + "users": [ + "owner" + ], + "roles": { + "owner": "owner" + }, + "teams": [], + "public": false, + "id": "layout", + "conditions": { + "0RA4iOShqVSy59V7Bb2w": { + "0RA4oxPNqnIjxANcPmqT": { + "id": "0RA4oxPNqnIjxANcPmqT", + "lastUpdated": 1698134929052, + "ifCondition": { + "id": "0RA4oxPNcyguHcHKdRan", + "lastUpdated": 1698134929052, + "expression": { + "leftPart": { + "valueString": "${role}", + "type": "Variable" + }, + "rightPart": { + "valueString": "Designer", + "type": "Variable" + }, + "operator": "equalsTo", + "type": "Expression" + }, + "actions": [ + { + "type": "setVariant", + "nodeID": "0RA4iOSkYO3SgMtPqrJq", + "variantID": "0RA4PZebIlc5dVbAOoAU" + } + ], + "mode": "ifMode", + "type": "Condition" + }, + "elseIfConditions": [], + "elseCondition": { + "id": "0RA4oxPMWhAHGXHbYa40", + "lastUpdated": 1698134929052, + "actions": [ + { + "type": "setVariant", + "nodeID": "0RA4iOSkYO3SgMtPqrJq", + "variantID": "default" + } + ], + "type": "ElseCondition" + }, + "name": "currentVariantId on node 0RA4iOSkYO3SgMtPqrJq", + "type": "ConditionGroup" + } + } + } + } +} \ No newline at end of file diff --git a/test/layout_loading_test.dart b/test/layout_loading_test.dart new file mode 100644 index 00000000..a1fa5248 --- /dev/null +++ b/test/layout_loading_test.dart @@ -0,0 +1,209 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:cloud_firestore/cloud_firestore.dart'; +import 'package:codelessly_sdk/codelessly_sdk.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:codelessly_sdk/src/ui/layout_builder.dart'; +import 'package:provider/provider.dart'; + +// A survey option: a variance whose "selected" variant is picked by a +// condition on `role`, which its "unselected" child sets on click. +const String canvasId = '0RA4iOShqVSy59V7Bb2w'; +const String selectedChild = '0RA4p4dcPLrhyyT2RYhQ'; + +final Map fixture = jsonDecode( + File('test/fixtures/option_variance.json').readAsStringSync(), +) as Map; + +SDKPublishLayout layout() => SDKPublishLayout.fromJson(fixture['layout']); + +SDKLayoutVariables variables() => + SDKLayoutVariables.fromJson(fixture['variables']); + +SDKLayoutConditions conditions() => + SDKLayoutConditions.fromJson(fixture['conditions']); + +SDKPublishModel publishModel() => SDKPublishModel( + projectId: 'project', + owner: 'owner', + teams: const {}, + users: const {}, + roles: const {}, + public: false, +); + +class _FakeCache extends Fake implements CacheManager {} + +class _FakeAuthManager extends Fake implements AuthManager { + @override + AuthData? get authData => AuthData( + authToken: 'token', + projectId: 'project', + timestamp: DateTime(2026), + owner: 'owner', + teams: const {}, + users: const {}, + roles: const {}, + ); +} + +class _FakeLocalRepository extends Fake implements LocalDataRepository {} + +class _FakeFirestore extends Fake implements FirebaseFirestore {} + +/// Serves the layout right away and its variables and conditions when +/// [release] completes. +class _SlowRepository extends Fake implements NetworkDataRepository { + final Completer release = Completer(); + + @override + Future downloadLayoutModel({ + required String layoutID, + required String projectID, + required PublishSource source, + }) async => layout(); + + @override + Future downloadLayoutVariables({ + required String projectID, + required String layoutID, + required PublishSource source, + }) async { + await release.future; + return variables(); + } + + @override + Future downloadLayoutConditions({ + required String projectID, + required String layoutID, + required PublishSource source, + }) async { + await release.future; + return conditions(); + } +} + +class _FakeLocalDatabase extends Fake implements LocalDatabase {} + +/// A data manager whose local storage is a fake, so layouts can render +/// without Hive. +class _TestDataManager extends DataManager { + _TestDataManager( + super.debugLabel, { + required super.config, + required super.cacheManager, + required super.authManager, + required super.networkDataRepository, + required super.localDataRepository, + required super.firebaseFirestore, + required super.errorLogger, + super.publishModel, + }); + + @override + LocalDatabase get localDatabase => _FakeLocalDatabase(); +} + +DataManager dataManager({ + SDKPublishModel? model, + NetworkDataRepository? network, +}) => _TestDataManager( + 'test', + config: CodelesslyConfig(authToken: 'token'), + cacheManager: _FakeCache(), + authManager: _FakeAuthManager(), + networkDataRepository: network ?? _SlowRepository(), + localDataRepository: _FakeLocalRepository(), + firebaseFirestore: _FakeFirestore(), + errorLogger: ErrorLogger.instance, + publishModel: model, +); + +void main() { + test( + 'a downloaded layout is added with its variables and conditions', + () async { + final model = publishModel().copyWith( + updates: SDKPublishUpdates( + layouts: {'layout': DateTime(2026)}, + variables: {'layout': DateTime(2026)}, + conditions: {'layout': DateTime(2026)}, + ), + ); + final network = _SlowRepository(); + final manager = dataManager(model: model, network: network); + + // Emitting the model needs local storage, which these fakes don't have, + // so the call fails after it has updated the model. + final Future fetch = manager + .getOrFetchPopulatedLayout(layoutID: 'layout') + .catchError((_) => false); + await pumpEventQueue(); + // While its variables and conditions download, a widget that rebuilds + // must not find the layout and render it without them. + expect(model.layouts.containsKey('layout'), isFalse); + + network.release.complete(); + await fetch; + expect(model.variables['layout'], isNotNull); + expect(model.conditions['layout'], isNotNull); + expect(model.layouts.containsKey('layout'), isTrue); + }, + ); + + testWidgets( + 'variables and conditions that arrive after the layout are loaded', + (tester) async { + final model = publishModel(); + model.layouts['layout'] = layout(); + final codelessly = Codelessly( + config: CodelesslyConfig(authToken: 'token'), + publishDataManager: dataManager(model: model), + ); + final controller = CodelesslyWidgetController( + layoutID: 'layout', + codelessly: codelessly, + ); + final codelesslyContext = CodelesslyContext.empty(layoutID: 'layout'); + + Widget app() => MultiProvider( + providers: [ + ChangeNotifierProvider.value( + value: codelesslyContext, + ), + Provider.value(value: codelessly), + ], + child: MaterialApp( + home: CodelesslyLayoutBuilder( + controller: controller, + layout: model.layouts['layout']!, + canvasId: canvasId, + ), + ), + ); + + // The layout is cached without its variables and conditions. + await tester.pumpWidget(app()); + expect(codelesslyContext.findVariableByName('role'), isNull); + + // They arrive and the publish model is emitted again. + model.variables['layout'] = variables(); + model.conditions['layout'] = conditions(); + await tester.pumpWidget(app()); + expect(codelesslyContext.findVariableByName('role'), isNotNull); + expect(codelesslyContext.conditions, isNotEmpty); + + await tester.tap(find.text('Designer').first, warnIfMissed: false); + await tester.pump(); + expect( + codelesslyContext.findVariableByName('role')!.value.value, + 'Designer', + ); + expect(find.byKey(const ValueKey(selectedChild)), findsWidgets); + }, + ); +}