You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by pa...@apache.org on 2022/08/09 15:06:51 UTC

[beam] branch master updated: Fix retaining unsaved pipeline options (#22075)

This is an automated email from the ASF dual-hosted git repository.

pabloem pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git


The following commit(s) were added to refs/heads/master by this push:
     new 3f328d6426c Fix retaining unsaved pipeline options (#22075)
     new 420494efd77 Merge pull request #22098 from akvelon/pg_22075_pipeline-options-fix
3f328d6426c is described below

commit 3f328d6426c0201140c82a784c0ff0fcccff572d
Author: Alexey Inkin <al...@akvelon.com>
AuthorDate: Tue Jun 28 17:50:28 2022 +0400

    Fix retaining unsaved pipeline options (#22075)
---
 ..._model.dart => pipeline_option_controller.dart} | 15 ++++++----
 .../pipeline_options_dropdown_body.dart            | 33 +++++++++++++---------
 .../pipeline_options_form.dart                     | 12 +++++---
 3 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_option_model.dart b/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_option_controller.dart
similarity index 73%
rename from playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_option_model.dart
rename to playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_option_controller.dart
index 8c8a0596718..21c59070465 100644
--- a/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_option_model.dart
+++ b/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_option_controller.dart
@@ -16,14 +16,19 @@
  * limitations under the License.
  */
 
-import 'package:flutter/cupertino.dart';
+import 'package:flutter/widgets.dart';
 
 class PipelineOptionController {
-  final TextEditingController name = TextEditingController();
-  final TextEditingController value = TextEditingController();
+  final TextEditingController nameController = TextEditingController();
+  final TextEditingController valueController = TextEditingController();
 
   PipelineOptionController({String name = '', String value = ''}) {
-    this.name.text = name;
-    this.value.text = value;
+    nameController.text = name;
+    valueController.text = value;
+  }
+
+  void dispose() {
+    nameController.dispose();
+    valueController.dispose();
   }
 }
diff --git a/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_options_dropdown_body.dart b/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_options_dropdown_body.dart
index f378ff2f72f..fd1d6c9eeda 100644
--- a/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_options_dropdown_body.dart
+++ b/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_options_dropdown_body.dart
@@ -20,7 +20,7 @@ import 'package:flutter/material.dart';
 import 'package:flutter_gen/gen_l10n/app_localizations.dart';
 import 'package:playground/constants/colors.dart';
 import 'package:playground/constants/sizes.dart';
-import 'package:playground/modules/editor/components/pipeline_options_dropdown/pipeline_option_model.dart';
+import 'package:playground/modules/editor/components/pipeline_options_dropdown/pipeline_option_controller.dart';
 import 'package:playground/modules/editor/components/pipeline_options_dropdown/pipeline_options_dropdown_input.dart';
 import 'package:playground/modules/editor/components/pipeline_options_dropdown/pipeline_options_dropdown_separator.dart';
 import 'package:playground/modules/editor/components/pipeline_options_dropdown/pipeline_options_form.dart';
@@ -29,19 +29,16 @@ import 'package:playground/modules/editor/parsers/run_options_parser.dart';
 const kOptionsTabIndex = 0;
 const kRawTabIndex = 1;
 
-final kDefaultOption = [PipelineOptionController()];
-
 class PipelineOptionsDropdownBody extends StatefulWidget {
   final String pipelineOptions;
   final void Function(String) setPipelineOptions;
   final void Function() close;
 
-  const PipelineOptionsDropdownBody({
-    Key? key,
+  PipelineOptionsDropdownBody({
     required this.pipelineOptions,
     required this.setPipelineOptions,
     required this.close,
-  }) : super(key: key);
+  }) : super(key: ValueKey(pipelineOptions));
 
   @override
   State<PipelineOptionsDropdownBody> createState() =>
@@ -54,7 +51,7 @@ class _PipelineOptionsDropdownBodyState
   late final TabController tabController;
   final TextEditingController pipelineOptionsController =
       TextEditingController();
-  List<PipelineOptionController> pipelineOptionsList = kDefaultOption;
+  List<PipelineOptionController> pipelineOptionsList = [];
   int selectedTab = kOptionsTabIndex;
   bool showError = false;
 
@@ -65,7 +62,7 @@ class _PipelineOptionsDropdownBodyState
     pipelineOptionsController.text = widget.pipelineOptions;
     pipelineOptionsList = _pipelineOptionsMapToList(widget.pipelineOptions);
     if (pipelineOptionsList.isEmpty) {
-      pipelineOptionsList = kDefaultOption;
+      pipelineOptionsList = [PipelineOptionController()];
     }
     super.initState();
   }
@@ -74,6 +71,12 @@ class _PipelineOptionsDropdownBodyState
   void dispose() {
     tabController.removeListener(onTabChange);
     tabController.dispose();
+    pipelineOptionsController.dispose();
+
+    for (final controller in pipelineOptionsList) {
+      controller.dispose();
+    }
+
     super.dispose();
   }
 
@@ -155,8 +158,8 @@ class _PipelineOptionsDropdownBodyState
                     appLocale.pipelineOptionsError,
                     style: Theme.of(context)
                         .textTheme
-                        .caption
-                        !.copyWith(color: kErrorNotificationColor),
+                        .caption!
+                        .copyWith(color: kErrorNotificationColor),
                     softWrap: true,
                   ),
                 ),
@@ -169,10 +172,14 @@ class _PipelineOptionsDropdownBodyState
 
   Map<String, String> get pipelineOptionsListValue {
     final notEmptyOptions = pipelineOptionsList
-        .where((option) =>
-            option.name.text.isNotEmpty && option.value.text.isNotEmpty)
+        .where((controller) =>
+            controller.nameController.text.isNotEmpty &&
+            controller.valueController.text.isNotEmpty)
         .toList();
-    return {for (var item in notEmptyOptions) item.name.text: item.value.text};
+    return {
+      for (final controller in notEmptyOptions)
+        controller.nameController.text: controller.valueController.text
+    };
   }
 
   String get pipelineOptionsValue {
diff --git a/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_options_form.dart b/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_options_form.dart
index fa72395dac0..a7f2eabf87e 100644
--- a/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_options_form.dart
+++ b/playground/frontend/lib/modules/editor/components/pipeline_options_dropdown/pipeline_options_form.dart
@@ -22,8 +22,8 @@ import 'package:collection/collection.dart';
 import 'package:playground/config/theme.dart';
 import 'package:playground/constants/colors.dart';
 import 'package:playground/constants/sizes.dart';
+import 'package:playground/modules/editor/components/pipeline_options_dropdown/pipeline_option_controller.dart';
 import 'package:playground/modules/editor/components/pipeline_options_dropdown/pipeline_option_label.dart';
-import 'package:playground/modules/editor/components/pipeline_options_dropdown/pipeline_option_model.dart';
 import 'package:playground/modules/editor/components/pipeline_options_dropdown/pipeline_options_text_field.dart';
 
 const kSpace = SizedBox(width: kMdSpacing);
@@ -53,19 +53,23 @@ class PipelineOptionsForm extends StatelessWidget {
           ],
         ),
         ...options.mapIndexed(
-          (index, option) => Row(
+          (index, controller) => Row(
             children: [
               Expanded(
                 child: SizedBox(
                   height: kTextFieldHeight,
-                  child: PipelineOptionsTextField(controller: option.name),
+                  child: PipelineOptionsTextField(
+                    controller: controller.nameController,
+                  ),
                 ),
               ),
               kSpace,
               Expanded(
                 child: SizedBox(
                   height: kTextFieldHeight,
-                  child: PipelineOptionsTextField(controller: option.value),
+                  child: PipelineOptionsTextField(
+                    controller: controller.valueController,
+                  ),
                 ),
               ),
               SizedBox(