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


##########
playground/frontend/lib/pages/standalone_playground/notifiers/example_selector_state.dart:
##########
@@ -47,28 +49,61 @@ class ExampleSelectorState with ChangeNotifier {
 
   void addSelectedTag(String tag) {
     selectedTags.add(tag);
+    _sortTagsBySelected();
     notifyListeners();
   }
 
   void removeSelectedTag(String tag) {
     selectedTags.remove(tag);
+    _sortTagsBySelected();
     notifyListeners();
   }
 
+  void _sortTagsBySelected() {
+    tags.sort((a, b) {
+      if (selectedTags.contains(a) && !selectedTags.contains(b)) {
+        return -1;
+      } else if (!selectedTags.contains(a) && selectedTags.contains(b)) {
+        return 1;
+      } else {
+        final aFreq = tagsFrequencyMap[a] ?? -1;
+        final bFreq = tagsFrequencyMap[b] ?? -1;
+        if (aFreq > bFreq) {
+          return -1;
+        } else if (aFreq < bFreq) {
+          return 1;
+        } else {
+          return a.compareTo(b);
+        }
+      }
+    });
+  }
+
   List<String> _getTagsSortedByExampleCount(
     List<CategoryWithExamples> categories,
   ) {
-    Map<String, int> exampleCountByTag = {};
+    Map<String, int> tagsFrequencyMap = _buildTagsFrequencyMap(categories);
+    final tagEntries = tagsFrequencyMap.entries.toList()
+      ..sort(
+        (entry1, entry2) => entry2.value.compareTo(entry1.value) != 0
+            ? entry2.value.compareTo(entry1.value)
+            : entry2.key.compareTo(entry1.key),
+      );
+    return tagEntries.map((entry) => entry.key).toList();
+  }
+
+  Map<String, int> _buildTagsFrequencyMap(
+    List<CategoryWithExamples> categories,
+  ) {
+    Map<String, int> result = {};

Review Comment:
   ```suggestion
       final result = <String, int>{};
   ```



##########
playground/frontend/lib/pages/standalone_playground/notifiers/example_selector_state.dart:
##########
@@ -47,28 +49,61 @@ class ExampleSelectorState with ChangeNotifier {
 
   void addSelectedTag(String tag) {
     selectedTags.add(tag);
+    _sortTagsBySelected();
     notifyListeners();
   }
 
   void removeSelectedTag(String tag) {
     selectedTags.remove(tag);
+    _sortTagsBySelected();
     notifyListeners();
   }
 
+  void _sortTagsBySelected() {

Review Comment:
   ```suggestion
     /// First selected, then most frequent, alphabetically for equal frequency.
     void _sortTags() {
   ```



##########
playground/frontend/playground_components/lib/src/controllers/code_runner.dart:
##########
@@ -122,8 +135,23 @@ class CodeRunner extends ChangeNotifier {
   }
 
   Future<void> cancelRun() async {
+    final hasInternet = (await Connectivity().checkConnectivity()).isConnected;
+    if (!hasInternet) {
+      _result = RunCodeResult(
+        status: _result?.status ?? RunCodeStatus.unspecified,
+        output: _result?.output,
+        log: _result?.log ?? '',
+        errorMessage: 'errors.internetUnavailable'.tr(),
+        graph: _result?.graph,
+      );
+      notifyListeners();
+      return;
+    }
+
     snippetEditingController = null;
-    await _runSubscription?.cancel();
+    // Awaited cancelling subscription here blocks futrher method execution. 

Review Comment:
   Typo.



##########
playground/frontend/playground_components/assets/translations/en.yaml:
##########
@@ -17,12 +17,16 @@
 
 errors:
   error: 'Error'
+  failedParseOptions: "Failed to parse pipeline options, please check the 
format (example: --key1 value1 --key2 value2), only alphanumeric and 
\",*,/,-,:,;,',. symbols are allowed"

Review Comment:
   We have both single and double quotes in this string, so we have to escape 
one of them anyway. Since all other strings in this file use single quotes, I 
suggest converting this one too.



##########
playground/frontend/playground_components/lib/src/repositories/code_client/grpc_code_client.dart:
##########
@@ -192,6 +196,9 @@ class GrpcCodeClient implements CodeClient {
     try {
       return await invoke();
     } on GrpcError catch (error) {
+      if (error.code == StatusCode.unknown) {
+        throw RunCodeError(message: 'errors.unknownError'.tr());
+      }

Review Comment:
   Explain why this is needed.



##########
playground/frontend/lib/pages/standalone_playground/notifiers/example_selector_state.dart:
##########
@@ -34,6 +35,7 @@ class ExampleSelectorState with ChangeNotifier {
     this._searchText = '',
   ]) {
     tags = _getTagsSortedByExampleCount(categories);

Review Comment:
   Can we reuse `_sortTags()` and delete this function?



##########
playground/frontend/lib/pages/standalone_playground/screen.dart:
##########
@@ -37,6 +37,7 @@ import 'widgets/playground_page_footer.dart';
 import 'widgets/playground_page_providers.dart';
 
 class StandalonePlaygroundScreen extends StatelessWidget {
+  static const kAppBarButtonsWidth = 1105;

Review Comment:
   Explain how this was obtained. Add a TODO and link to an issue to do some 
automatic calculation.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to