You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "alexeyinkin (via GitHub)" <gi...@apache.org> on 2023/05/11 08:40:55 UTC

[GitHub] [beam] alexeyinkin commented on a diff in pull request #26630: [Tour of Beam] [Frontend] Unit navigation draft

alexeyinkin commented on code in PR #26630:
URL: https://github.com/apache/beam/pull/26630#discussion_r1190821403


##########
learning/tour-of-beam/frontend/lib/pages/tour/controllers/content_tree.dart:
##########
@@ -29,25 +29,27 @@ import '../../../state.dart';
 
 class ContentTreeController extends ChangeNotifier {
   final Sdk initialSdk;
-  List<String> _treeIds;
+  List<String> _breadcrumbs;

Review Comment:
   ```suggestion
     List<String> _breadcrumbIds;
   ```



##########
learning/tour-of-beam/frontend/assets/translations/en.yaml:
##########
@@ -51,6 +51,8 @@ pages:
     solveYourself: Before revealing the solution, try solving the challenge on your own. Remember, the more you practice, the better you will become. Give it a shot and see how far you can get.
     example: Example
     myCode: My code
+    nextUnit: Next unit
+    previousUnit: Previous unit

Review Comment:
   ```suggestion
       nextUnit: Next Unit
       previousUnit: Previous Unit
   ```



##########
learning/tour-of-beam/frontend/lib/models/content_tree.dart:
##########
@@ -20,29 +20,50 @@ import '../repositories/models/get_content_tree_response.dart';
 import 'module.dart';
 import 'node.dart';
 import 'parent_node.dart';
+import 'unit.dart';
 
 class ContentTreeModel extends ParentNodeModel {
   final List<ModuleModel> modules;
 
-  String get sdkId => id;
+  final List<UnitModel> units;
 
-  @override
-  List<NodeModel> get nodes => modules;
+  String get sdkId => id;
 
   const ContentTreeModel({
     required super.id,
     required this.modules,
+    required this.units,
   }) : super(
           parent: null,
           title: '',
           nodes: modules,
         );
 
+  static List<UnitModel> _getUnitsFromModules(List<ModuleModel> modules) {
+    final units = <UnitModel>[];
+
+    void extractUnitsFromNode(NodeModel node) {
+      if (node is ParentNodeModel) {
+        node.nodes.forEach(extractUnitsFromNode);
+      } else if (node is UnitModel) {
+        units.add(node);
+      }
+    }
+
+    modules.forEach(extractUnitsFromNode);
+    return units;
+  }

Review Comment:
   To `ParentNodeModel.asList()`



##########
learning/tour-of-beam/frontend/lib/pages/tour/controllers/content_tree.dart:
##########
@@ -110,12 +112,44 @@ class ContentTreeController extends ChangeNotifier {
     }
 
     _toggleNode(
-      contentTree.getNodeByTreeIds(_treeIds) ?? contentTree.modules.first,
+      contentTree.getNodeByTreeIds(_breadcrumbs) ?? contentTree.modules.first,
     );
 
+    units.clear();
+    units.addAll(contentTree.getUnitsFromModules());
+
     notifyListeners();
   }
 
+  bool hasPreviousUnit() {
+    return _getCurrentUnitIndex() > 0;
+  }
+
+  bool hasNextUnit() {
+    return _getCurrentUnitIndex() < units.length - 1;
+  }
+
+  void openPreviousUnit() {
+    final previousUnit = units[_getCurrentUnitIndex() - 1];
+    _navigateToUnit(previousUnit);
+  }
+
+  void openNextUnit() {
+    final nextUnit = units[_getCurrentUnitIndex() + 1];
+    _navigateToUnit(nextUnit);
+  }
+
+  int _getCurrentUnitIndex() {

Review Comment:
   Cache the current index on any navigation or tree update.



##########
learning/tour-of-beam/frontend/lib/models/content_tree.dart:
##########
@@ -38,6 +36,21 @@ class ContentTreeModel extends ParentNodeModel {
           nodes: modules,
         );
 
+  List<UnitModel> getUnitsFromModules() {

Review Comment:
   To `ParentNodeModel.asList()`



##########
learning/tour-of-beam/frontend/lib/pages/tour/state.dart:
##########
@@ -60,7 +59,8 @@ class TourNotifier extends ChangeNotifier with PageStateMixin<void> {
     List<String> initialTreeIds = const [],
   })  : contentTreeController = ContentTreeController(
           initialSdk: initialSdk,
-          initialTreeIds: initialTreeIds,
+          // TODO(nausharipov) review: replace treeIds with breadcrumbs everywhere?

Review Comment:
   Yes.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org