You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2023/01/03 14:36:46 UTC
[GitHub] [beam] alexeyinkin opened a new pull request, #24865: Multifile examples on frontend (#24859)
alexeyinkin opened a new pull request, #24865:
URL: https://github.com/apache/beam/pull/24865
- Resolves #24859
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/get-started-contributing/#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063214480
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -156,19 +85,32 @@ class SnippetEditingController extends ChangeNotifier {
bool get isChanged => _isChanged;
void _updateIsChanged() {
- _isChanged = _isCodeChanged() || _arePipelineOptionsChanged();
+ _isChanged = _calculateIsChanged();
}
- bool _isCodeChanged() {
- return _selectedExample?.source != codeController.fullText;
+ bool _calculateIsChanged() {
+ for (final controller in fileControllers) {
+ if (controller.isChanged) {
+ return true;
+ }
+ }
+
+ if (_arePipelineOptionsChanged()) {
+ return true;
+ }
+
+ return false;
}
bool _arePipelineOptionsChanged() {
return _pipelineOptions != (_selectedExample?.pipelineOptions ?? '');
}
void reset() {
- codeController.text = _selectedExample?.source ?? '';
+ for (final controller in fileControllers) {
Review Comment:
`forEach` is only worthy with a tear off like:
```dart
void process(obj) { ... }
collection.forEach(process);
```
Otherwise it is not much shorter than an ordinary `for` but:
- Requires a lambda like `(e) => e.method()` to be constructed which may be less performant.
- Adds two extra stack frames which is harder to debug.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063230986
##########
playground/frontend/playground_components/lib/src/controllers/snippet_file_editing_controller.dart:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'dart:math';
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_code_editor/flutter_code_editor.dart';
+import 'package:get_it/get_it.dart';
+
+import '../models/example_view_options.dart';
+import '../models/sdk.dart';
+import '../models/snippet_file.dart';
+import '../services/symbols/symbols_notifier.dart';
+
+/// The main state object for a file in a snippet.
+class SnippetFileEditingController extends ChangeNotifier {
+ final CodeController codeController;
+ final SnippetFile savedFile;
+ final Sdk sdk;
+
+ bool _isChanged = false;
+
+ final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
+ SnippetFileEditingController({
+ required this.savedFile,
+ required this.sdk,
+ required ExampleViewOptions viewOptions,
+ int? contextLine1Based,
+ }) : codeController = CodeController(
+ language: sdk.highlightMode,
+ namedSectionParser: const BracketsStartEndNamedSectionParser(),
+ text: savedFile.content,
+ ) {
+ _applyViewOptions(viewOptions);
+ if (contextLine1Based != null) {
+ _toStartOfFullLine(max(contextLine1Based - 1, 0));
+ }
+
+ codeController.addListener(_onCodeControllerChanged);
+ _symbolsNotifier.addListener(_onSymbolsNotifierChanged);
+ _onSymbolsNotifierChanged();
+ }
+
+ void _applyViewOptions(ExampleViewOptions options) {
+ codeController.readOnlySectionNames = options.readOnlySectionNames.toSet();
+ codeController.visibleSectionNames = options.showSectionNames.toSet();
+
+ if (options.foldCommentAtLineZero) {
+ codeController.foldCommentAtLineZero();
+ }
+
+ if (options.foldImports) {
+ codeController.foldImports();
+ }
+
+ final unfolded = options.unfoldSectionNames;
+ if (unfolded.isNotEmpty) {
+ codeController.foldOutsideSections(unfolded);
+ }
+ }
+
+ void _toStartOfFullLine(int line) {
Review Comment:
"To start" of what then?
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063227920
##########
playground/frontend/playground_components/lib/src/controllers/playground_controller.dart:
##########
@@ -400,22 +393,19 @@ class PlaygroundController with ChangeNotifier {
}
Future<UserSharedExampleLoadingDescriptor> saveSnippet() async {
- final controller = requireSnippetEditingController();
- final code = controller.codeController.fullText;
- final name = 'examples.userSharedName'.tr();
+ final snippetController = requireSnippetEditingController();
+ final files = snippetController.getFiles();
final snippetId = await exampleCache.saveSnippet(
- files: [
- SharedFile(code: code, isMain: true, name: name),
- ],
- sdk: controller.sdk,
- pipelineOptions: controller.pipelineOptions,
+ files: files,
+ sdk: snippetController.sdk,
+ pipelineOptions: snippetController.pipelineOptions,
);
final sharedExample = Example(
- source: code,
- name: name,
- sdk: controller.sdk,
+ files: files,
+ name: files.first.name,
Review Comment:
Not easily. We only show the sharing button if there are any.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063203680
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -65,3 +59,21 @@ class ExampleItemActions extends StatelessWidget {
Provider.of<PopoverState>(context, listen: false).setOpen(isOpen);
}
}
+
+/// A wrapper of a standard size for icons in the example list.
+class _Icon extends StatelessWidget {
Review Comment:
`+`
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063216932
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -156,19 +85,32 @@ class SnippetEditingController extends ChangeNotifier {
bool get isChanged => _isChanged;
void _updateIsChanged() {
- _isChanged = _isCodeChanged() || _arePipelineOptionsChanged();
+ _isChanged = _calculateIsChanged();
}
- bool _isCodeChanged() {
- return _selectedExample?.source != codeController.fullText;
+ bool _calculateIsChanged() {
+ for (final controller in fileControllers) {
+ if (controller.isChanged) {
+ return true;
+ }
+ }
+
+ if (_arePipelineOptionsChanged()) {
+ return true;
+ }
+
+ return false;
}
Review Comment:
`+`
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063206881
##########
playground/frontend/playground_components/lib/src/controllers/playground_controller.dart:
##########
@@ -400,22 +393,19 @@ class PlaygroundController with ChangeNotifier {
}
Future<UserSharedExampleLoadingDescriptor> saveSnippet() async {
- final controller = requireSnippetEditingController();
- final code = controller.codeController.fullText;
- final name = 'examples.userSharedName'.tr();
+ final snippetController = requireSnippetEditingController();
+ final files = snippetController.getFiles();
final snippetId = await exampleCache.saveSnippet(
- files: [
- SharedFile(code: code, isMain: true, name: name),
- ],
- sdk: controller.sdk,
- pipelineOptions: controller.pipelineOptions,
+ files: files,
+ sdk: snippetController.sdk,
+ pipelineOptions: snippetController.pipelineOptions,
);
final sharedExample = Example(
- source: code,
- name: name,
- sdk: controller.sdk,
+ files: files,
Review Comment:
`+`
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063232470
##########
playground/frontend/playground_components/lib/src/controllers/snippet_file_editing_controller.dart:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'dart:math';
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_code_editor/flutter_code_editor.dart';
+import 'package:get_it/get_it.dart';
+
+import '../models/example_view_options.dart';
+import '../models/sdk.dart';
+import '../models/snippet_file.dart';
+import '../services/symbols/symbols_notifier.dart';
+
+/// The main state object for a file in a snippet.
+class SnippetFileEditingController extends ChangeNotifier {
+ final CodeController codeController;
+ final SnippetFile savedFile;
+ final Sdk sdk;
+
+ bool _isChanged = false;
+
+ final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
+ SnippetFileEditingController({
+ required this.savedFile,
+ required this.sdk,
+ required ExampleViewOptions viewOptions,
+ int? contextLine1Based,
+ }) : codeController = CodeController(
+ language: sdk.highlightMode,
+ namedSectionParser: const BracketsStartEndNamedSectionParser(),
+ text: savedFile.content,
+ ) {
+ _applyViewOptions(viewOptions);
+ if (contextLine1Based != null) {
+ _toStartOfFullLine(max(contextLine1Based - 1, 0));
+ }
+
+ codeController.addListener(_onCodeControllerChanged);
+ _symbolsNotifier.addListener(_onSymbolsNotifierChanged);
+ _onSymbolsNotifierChanged();
+ }
+
+ void _applyViewOptions(ExampleViewOptions options) {
+ codeController.readOnlySectionNames = options.readOnlySectionNames.toSet();
+ codeController.visibleSectionNames = options.showSectionNames.toSet();
+
+ if (options.foldCommentAtLineZero) {
+ codeController.foldCommentAtLineZero();
+ }
+
+ if (options.foldImports) {
+ codeController.foldImports();
+ }
+
+ final unfolded = options.unfoldSectionNames;
+ if (unfolded.isNotEmpty) {
+ codeController.foldOutsideSections(unfolded);
+ }
+ }
+
+ void _toStartOfFullLine(int line) {
+ if (line >= codeController.code.lines.length) {
+ return;
+ }
+
+ final fullPosition = codeController.code.lines.lines[line].textRange.start;
+ final visiblePosition = codeController.code.hiddenRanges.cutPosition(
+ fullPosition,
+ );
+
+ codeController.selection = TextSelection.collapsed(
+ offset: visiblePosition,
+ );
+ }
+
+ void _onCodeControllerChanged() {
+ if (!_isChanged) {
Review Comment:
The idea is that we may later have other properties that will also be reasons for change. The current `_isChanged` will remain broad to account for them all, and this is what meant to be checked here.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063224726
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -65,3 +59,21 @@ class ExampleItemActions extends StatelessWidget {
Provider.of<PopoverState>(context, listen: false).setOpen(isOpen);
}
}
+
+/// A wrapper of a standard size for icons in the example list.
+class _Icon extends StatelessWidget {
+ const _Icon(this.child);
+
+ final Widget child;
+
+ @override
+ Widget build(BuildContext context) {
+ return SizedBox(
+ width: _iconSize,
Review Comment:
`+`
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063230249
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -156,19 +85,32 @@ class SnippetEditingController extends ChangeNotifier {
bool get isChanged => _isChanged;
void _updateIsChanged() {
- _isChanged = _isCodeChanged() || _arePipelineOptionsChanged();
+ _isChanged = _calculateIsChanged();
}
- bool _isCodeChanged() {
- return _selectedExample?.source != codeController.fullText;
+ bool _calculateIsChanged() {
+ for (final controller in fileControllers) {
Review Comment:
Local unambiguous variables are better short.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063223849
##########
playground/frontend/playground_components/lib/src/controllers/snippet_file_editing_controller.dart:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'dart:math';
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_code_editor/flutter_code_editor.dart';
+import 'package:get_it/get_it.dart';
+
+import '../models/example_view_options.dart';
+import '../models/sdk.dart';
+import '../models/snippet_file.dart';
+import '../services/symbols/symbols_notifier.dart';
+
+/// The main state object for a file in a snippet.
+class SnippetFileEditingController extends ChangeNotifier {
+ final CodeController codeController;
+ final SnippetFile savedFile;
+ final Sdk sdk;
+
+ bool _isChanged = false;
+
+ final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
+ SnippetFileEditingController({
+ required this.savedFile,
+ required this.sdk,
+ required ExampleViewOptions viewOptions,
+ int? contextLine1Based,
+ }) : codeController = CodeController(
+ language: sdk.highlightMode,
+ namedSectionParser: const BracketsStartEndNamedSectionParser(),
+ text: savedFile.content,
+ ) {
+ _applyViewOptions(viewOptions);
+ if (contextLine1Based != null) {
Review Comment:
`+`
--
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
[GitHub] [beam] Malarg commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
Malarg commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1061182400
##########
playground/frontend/lib/pages/standalone_playground/widgets/editor_textarea_wrapper.dart:
##########
@@ -47,60 +46,34 @@ class CodeTextAreaWrapper extends StatelessWidget {
return const LoadingIndicator();
}
- return Column(
- children: [
- Expanded(
- child: Stack(
- children: [
- Positioned.fill(
- child: SnippetEditor(
- controller: snippetController,
- isEditable: true,
- ),
+ return SnippetEditor(
+ controller: snippetController,
+ isEditable: true,
+ actionsWidget: Row(
Review Comment:
Suggest to extract as a stateless widget
##########
playground/frontend/playground_components/lib/src/controllers/example_loaders/http_example_loader.dart:
##########
@@ -48,9 +49,11 @@ class HttpExampleLoader extends ExampleLoader {
return Example(
name: descriptor.uri.path.split('/').lastOrNull ?? 'HTTP Example',
+ files: [
Review Comment:
http loader does not support multifile?
##########
playground/frontend/playground_components/lib/src/controllers/playground_controller.dart:
##########
@@ -400,22 +393,19 @@ class PlaygroundController with ChangeNotifier {
}
Future<UserSharedExampleLoadingDescriptor> saveSnippet() async {
- final controller = requireSnippetEditingController();
- final code = controller.codeController.fullText;
- final name = 'examples.userSharedName'.tr();
+ final snippetController = requireSnippetEditingController();
+ final files = snippetController.getFiles();
final snippetId = await exampleCache.saveSnippet(
- files: [
- SharedFile(code: code, isMain: true, name: name),
- ],
- sdk: controller.sdk,
- pipelineOptions: controller.pipelineOptions,
+ files: files,
Review Comment:
Sort.
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -156,19 +85,32 @@ class SnippetEditingController extends ChangeNotifier {
bool get isChanged => _isChanged;
void _updateIsChanged() {
- _isChanged = _isCodeChanged() || _arePipelineOptionsChanged();
+ _isChanged = _calculateIsChanged();
}
- bool _isCodeChanged() {
- return _selectedExample?.source != codeController.fullText;
+ bool _calculateIsChanged() {
+ for (final controller in fileControllers) {
+ if (controller.isChanged) {
+ return true;
+ }
+ }
+
+ if (_arePipelineOptionsChanged()) {
+ return true;
+ }
+
+ return false;
}
Review Comment:
```suggestion
bool _calculateIsChanged() {
return _isAnyFileControllerChanged() || _arePipelineOptionsChanged();
}
```
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -186,39 +128,84 @@ class SnippetEditingController extends ChangeNotifier {
return ContentExampleLoadingDescriptor(
complexity: example.complexity,
- content: codeController.fullText,
+ files: getFiles(),
name: example.name,
sdk: sdk,
);
}
- void setSource(String source) {
- codeController.readOnlySectionNames = const {};
- codeController.visibleSectionNames = const {};
+ void _replaceFileControllers(
Review Comment:
Pretty big method. Could be splitted on `_dismantleOldControllers` and `_buildNewControllers`
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -16,54 +16,33 @@
* limitations under the License.
*/
-import 'dart:math';
-
+import 'package:collection/collection.dart';
import 'package:flutter/widgets.dart';
-import 'package:flutter_code_editor/flutter_code_editor.dart';
-import 'package:get_it/get_it.dart';
import '../models/example.dart';
import '../models/example_loading_descriptors/content_example_loading_descriptor.dart';
import '../models/example_loading_descriptors/empty_example_loading_descriptor.dart';
import '../models/example_loading_descriptors/example_loading_descriptor.dart';
import '../models/example_view_options.dart';
import '../models/sdk.dart';
-import '../services/symbols/symbols_notifier.dart';
+import '../models/snippet_file.dart';
+import 'snippet_file_editing_controller.dart';
/// The main state object for a single [sdk].
class SnippetEditingController extends ChangeNotifier {
+ final List<SnippetFileEditingController> fileControllers = [];
final Sdk sdk;
- final CodeController codeController;
- final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
Example? _selectedExample;
Review Comment:
Sort?
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -186,39 +128,84 @@ class SnippetEditingController extends ChangeNotifier {
return ContentExampleLoadingDescriptor(
complexity: example.complexity,
- content: codeController.fullText,
+ files: getFiles(),
name: example.name,
sdk: sdk,
);
}
- void setSource(String source) {
- codeController.readOnlySectionNames = const {};
- codeController.visibleSectionNames = const {};
+ void _replaceFileControllers(
+ Iterable<SnippetFile> files,
+ ExampleViewOptions viewOptions,
+ ) {
+ for (final oldController in fileControllers) {
+ oldController.removeListener(_onFileControllerChanged);
+ }
+ final newControllers = <SnippetFileEditingController>[];
+
+ for (final file in files) {
+ final controller = SnippetFileEditingController(
+ contextLine1Based: file.isMain ? _selectedExample?.contextLine : null,
+ savedFile: file,
+ sdk: sdk,
+ viewOptions: viewOptions,
+ );
+
+ newControllers.add(controller);
+ controller.addListener(_onFileControllerChanged);
+ }
- codeController.fullText = source;
- codeController.historyController.deleteHistory();
- }
+ for (final oldController in fileControllers) {
+ oldController.dispose();
+ }
- void _onSymbolsNotifierChanged() {
- final mode = sdk.highlightMode;
- if (mode == null) {
- return;
+ fileControllers.clear();
+ fileControllers.addAll(newControllers);
+
+ _fileControllersByName.clear();
+ for (final controller in newControllers) {
+ _fileControllersByName[controller.savedFile.name] = controller;
}
- final dictionary = _symbolsNotifier.getDictionary(mode);
- if (dictionary == null) {
- return;
+ _activeFileController =
+ fileControllers.firstWhereOrNull((c) => c.savedFile.isMain);
+ }
+
+ void _onFileControllerChanged() {
+ if (!_isChanged) {
+ if (_isAnyFileControllerChanged()) {
+ _isChanged = true;
+ notifyListeners();
+ }
+ } else {
+ _updateIsChanged();
+ if (!_isChanged) {
+ notifyListeners();
+ }
}
+ }
- codeController.autocompleter.setCustomWords(dictionary.symbols);
+ bool _isAnyFileControllerChanged() {
+ return fileControllers.any((c) => c.isChanged);
}
- @override
- void dispose() {
- _symbolsNotifier.removeListener(
- _onSymbolsNotifierChanged,
- );
- super.dispose();
+ SnippetFileEditingController? get activeFileController =>
+ _activeFileController;
+
+ SnippetFileEditingController? getFileControllerByName(String name) {
+ return _fileControllersByName[name];
+ }
+
+ void activateFileControllerByName(String name) {
+ final newController = getFileControllerByName(name);
Review Comment:
Are there situation possible if newController is null here?
##########
playground/frontend/playground_components/lib/src/repositories/example_client/grpc_example_client.dart:
##########
@@ -364,7 +369,7 @@ class GrpcExampleClient implements ExampleClient {
grpc.SnippetFile()
..name = item.name
..isMain = true
- ..content = item.code,
+ ..content = item.content,
Review Comment:
```
grpc.SnippetFile(
content: item.content,
isMain: true,
name: item.name,
),
```
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -186,39 +128,84 @@ class SnippetEditingController extends ChangeNotifier {
return ContentExampleLoadingDescriptor(
complexity: example.complexity,
- content: codeController.fullText,
+ files: getFiles(),
name: example.name,
sdk: sdk,
);
}
- void setSource(String source) {
- codeController.readOnlySectionNames = const {};
- codeController.visibleSectionNames = const {};
+ void _replaceFileControllers(
+ Iterable<SnippetFile> files,
+ ExampleViewOptions viewOptions,
+ ) {
+ for (final oldController in fileControllers) {
+ oldController.removeListener(_onFileControllerChanged);
+ }
+ final newControllers = <SnippetFileEditingController>[];
+
+ for (final file in files) {
+ final controller = SnippetFileEditingController(
+ contextLine1Based: file.isMain ? _selectedExample?.contextLine : null,
+ savedFile: file,
+ sdk: sdk,
+ viewOptions: viewOptions,
+ );
+
+ newControllers.add(controller);
+ controller.addListener(_onFileControllerChanged);
+ }
- codeController.fullText = source;
- codeController.historyController.deleteHistory();
- }
+ for (final oldController in fileControllers) {
+ oldController.dispose();
+ }
- void _onSymbolsNotifierChanged() {
- final mode = sdk.highlightMode;
- if (mode == null) {
- return;
+ fileControllers.clear();
+ fileControllers.addAll(newControllers);
+
+ _fileControllersByName.clear();
+ for (final controller in newControllers) {
+ _fileControllersByName[controller.savedFile.name] = controller;
}
- final dictionary = _symbolsNotifier.getDictionary(mode);
- if (dictionary == null) {
- return;
+ _activeFileController =
+ fileControllers.firstWhereOrNull((c) => c.savedFile.isMain);
+ }
+
+ void _onFileControllerChanged() {
+ if (!_isChanged) {
+ if (_isAnyFileControllerChanged()) {
+ _isChanged = true;
+ notifyListeners();
+ }
+ } else {
+ _updateIsChanged();
+ if (!_isChanged) {
+ notifyListeners();
+ }
}
+ }
- codeController.autocompleter.setCustomWords(dictionary.symbols);
+ bool _isAnyFileControllerChanged() {
Review Comment:
`_areFileControllersChanged`
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -65,3 +59,21 @@ class ExampleItemActions extends StatelessWidget {
Provider.of<PopoverState>(context, listen: false).setOpen(isOpen);
}
}
+
+/// A wrapper of a standard size for icons in the example list.
+class _Icon extends StatelessWidget {
+ const _Icon(this.child);
Review Comment:
rename to `icon` and make parameter named?
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -156,19 +85,32 @@ class SnippetEditingController extends ChangeNotifier {
bool get isChanged => _isChanged;
void _updateIsChanged() {
- _isChanged = _isCodeChanged() || _arePipelineOptionsChanged();
+ _isChanged = _calculateIsChanged();
}
- bool _isCodeChanged() {
- return _selectedExample?.source != codeController.fullText;
+ bool _calculateIsChanged() {
+ for (final controller in fileControllers) {
+ if (controller.isChanged) {
+ return true;
+ }
+ }
+
+ if (_arePipelineOptionsChanged()) {
+ return true;
+ }
+
+ return false;
}
bool _arePipelineOptionsChanged() {
return _pipelineOptions != (_selectedExample?.pipelineOptions ?? '');
}
void reset() {
- codeController.text = _selectedExample?.source ?? '';
+ for (final controller in fileControllers) {
Review Comment:
tried to change this on `forEach`, but we have a linter rule, forbidding this. To me, `forEach` method less verbose here. What do you think?
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -17,12 +17,15 @@
*/
import 'package:flutter/material.dart';
-import 'package:playground/modules/examples/components/description_popover/description_popover_button.dart';
-import 'package:playground/modules/examples/components/multifile_popover/multifile_popover_button.dart';
-import 'package:playground/modules/examples/models/popover_state.dart';
import 'package:playground_components/playground_components.dart';
import 'package:provider/provider.dart';
+import '../../models/popover_state.dart';
+import '../description_popover/description_popover_button.dart';
+import '../multi_file_icon.dart';
+
+const double _iconSize = 30;
Review Comment:
1. Move to `_Icon`? Looks like it doesn't required in the whole file.
2. May be to use `kIconSizeLg` instead of this variable? Especially `DescriptionPopoverButton` uses it
##########
playground/frontend/playground_components/lib/src/controllers/playground_controller.dart:
##########
@@ -400,22 +393,19 @@ class PlaygroundController with ChangeNotifier {
}
Future<UserSharedExampleLoadingDescriptor> saveSnippet() async {
- final controller = requireSnippetEditingController();
- final code = controller.codeController.fullText;
- final name = 'examples.userSharedName'.tr();
+ final snippetController = requireSnippetEditingController();
+ final files = snippetController.getFiles();
final snippetId = await exampleCache.saveSnippet(
- files: [
- SharedFile(code: code, isMain: true, name: name),
- ],
- sdk: controller.sdk,
- pipelineOptions: controller.pipelineOptions,
+ files: files,
+ sdk: snippetController.sdk,
+ pipelineOptions: snippetController.pipelineOptions,
);
final sharedExample = Example(
- source: code,
- name: name,
- sdk: controller.sdk,
+ files: files,
Review Comment:
Sort
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -65,3 +59,21 @@ class ExampleItemActions extends StatelessWidget {
Provider.of<PopoverState>(context, listen: false).setOpen(isOpen);
}
}
+
+/// A wrapper of a standard size for icons in the example list.
+class _Icon extends StatelessWidget {
Review Comment:
`_IconFrame` or `_IconWrapper`?
##########
playground/frontend/playground_components/lib/src/controllers/snippet_file_editing_controller.dart:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'dart:math';
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_code_editor/flutter_code_editor.dart';
+import 'package:get_it/get_it.dart';
+
+import '../models/example_view_options.dart';
+import '../models/sdk.dart';
+import '../models/snippet_file.dart';
+import '../services/symbols/symbols_notifier.dart';
+
+/// The main state object for a file in a snippet.
+class SnippetFileEditingController extends ChangeNotifier {
+ final CodeController codeController;
+ final SnippetFile savedFile;
+ final Sdk sdk;
+
+ bool _isChanged = false;
+
+ final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
+ SnippetFileEditingController({
+ required this.savedFile,
+ required this.sdk,
+ required ExampleViewOptions viewOptions,
+ int? contextLine1Based,
+ }) : codeController = CodeController(
+ language: sdk.highlightMode,
+ namedSectionParser: const BracketsStartEndNamedSectionParser(),
+ text: savedFile.content,
+ ) {
+ _applyViewOptions(viewOptions);
+ if (contextLine1Based != null) {
Review Comment:
why this logic should be here? Would be awesome if comment will be added here
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063220344
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -186,39 +128,84 @@ class SnippetEditingController extends ChangeNotifier {
return ContentExampleLoadingDescriptor(
complexity: example.complexity,
- content: codeController.fullText,
+ files: getFiles(),
name: example.name,
sdk: sdk,
);
}
- void setSource(String source) {
- codeController.readOnlySectionNames = const {};
- codeController.visibleSectionNames = const {};
+ void _replaceFileControllers(
+ Iterable<SnippetFile> files,
+ ExampleViewOptions viewOptions,
+ ) {
+ for (final oldController in fileControllers) {
+ oldController.removeListener(_onFileControllerChanged);
+ }
+ final newControllers = <SnippetFileEditingController>[];
+
+ for (final file in files) {
+ final controller = SnippetFileEditingController(
+ contextLine1Based: file.isMain ? _selectedExample?.contextLine : null,
+ savedFile: file,
+ sdk: sdk,
+ viewOptions: viewOptions,
+ );
+
+ newControllers.add(controller);
+ controller.addListener(_onFileControllerChanged);
+ }
- codeController.fullText = source;
- codeController.historyController.deleteHistory();
- }
+ for (final oldController in fileControllers) {
+ oldController.dispose();
+ }
- void _onSymbolsNotifierChanged() {
- final mode = sdk.highlightMode;
- if (mode == null) {
- return;
+ fileControllers.clear();
+ fileControllers.addAll(newControllers);
+
+ _fileControllersByName.clear();
+ for (final controller in newControllers) {
+ _fileControllersByName[controller.savedFile.name] = controller;
}
- final dictionary = _symbolsNotifier.getDictionary(mode);
- if (dictionary == null) {
- return;
+ _activeFileController =
+ fileControllers.firstWhereOrNull((c) => c.savedFile.isMain);
+ }
+
+ void _onFileControllerChanged() {
+ if (!_isChanged) {
+ if (_isAnyFileControllerChanged()) {
+ _isChanged = true;
+ notifyListeners();
+ }
+ } else {
+ _updateIsChanged();
+ if (!_isChanged) {
+ notifyListeners();
+ }
}
+ }
- codeController.autocompleter.setCustomWords(dictionary.symbols);
+ bool _isAnyFileControllerChanged() {
Review Comment:
How many of them?
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063225849
##########
playground/frontend/lib/pages/standalone_playground/widgets/editor_textarea_wrapper.dart:
##########
@@ -47,60 +46,34 @@ class CodeTextAreaWrapper extends StatelessWidget {
return const LoadingIndicator();
}
- return Column(
- children: [
- Expanded(
- child: Stack(
- children: [
- Positioned.fill(
- child: SnippetEditor(
- controller: snippetController,
- isEditable: true,
- ),
+ return SnippetEditor(
+ controller: snippetController,
+ isEditable: true,
+ actionsWidget: Row(
+ children: [
+ if (controller.selectedExample != null)
+ Semantics(
+ container: true,
+ child: DescriptionPopoverButton(
+ example: controller.selectedExample!,
Review Comment:
`+`
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063237431
##########
playground/frontend/playground_components/lib/src/widgets/snippet_editor.dart:
##########
@@ -16,120 +16,69 @@
* limitations under the License.
*/
-import 'dart:math';
-
import 'package:flutter/material.dart';
-import 'package:flutter/scheduler.dart';
-import 'package:flutter_code_editor/flutter_code_editor.dart';
+import '../constants/sizes.dart';
import '../controllers/snippet_editing_controller.dart';
-import '../theme/theme.dart';
-
-class SnippetEditor extends StatefulWidget {
- final SnippetEditingController controller;
- final bool isEditable;
+import 'loading_indicator.dart';
+import 'snippet_file_editor.dart';
+import 'tabbed_snippet_editor.dart';
- SnippetEditor({
+class SnippetEditor extends StatelessWidget {
+ const SnippetEditor({
required this.controller,
required this.isEditable,
- }) : super(
- // When the example is changed, will scroll to the context line again.
- key: ValueKey(controller.selectedExample),
- );
-
- @override
- State<SnippetEditor> createState() => _SnippetEditorState();
-}
-
-class _SnippetEditorState extends State<SnippetEditor> {
- bool _didAutoFocus = false;
- final _focusNode = FocusNode();
- final _scrollController = ScrollController();
-
- @override
- void didChangeDependencies() {
- super.didChangeDependencies();
-
- if (!_didAutoFocus) {
- _didAutoFocus = true;
- SchedulerBinding.instance.addPostFrameCallback((_) {
- if (mounted) {
- _scrollSoCursorIsOnTop();
- }
- });
- }
- }
-
- void _scrollSoCursorIsOnTop() {
- _focusNode.requestFocus();
+ this.actionsWidget,
+ });
- final position = max(widget.controller.codeController.selection.start, 0);
- final characterOffset = _getLastCharacterOffset(
- text: widget.controller.codeController.text.substring(0, position),
- style: kLightTheme.extension<BeamThemeExtension>()!.codeRootStyle,
- );
+ final SnippetEditingController controller;
+ final bool isEditable;
- _scrollController.jumpTo(
- min(
- characterOffset.dy,
- _scrollController.position.maxScrollExtent,
- ),
- );
- }
-
- @override
- void dispose() {
- _focusNode.dispose();
- super.dispose();
- }
+ /// A child widget that will be:
+ /// - Hidden if no file is loaded.
+ /// - Shown as an overlay for a single file editor.
+ /// - Built into the tab bar for a multi-file editor.
+ final Widget? actionsWidget;
@override
Widget build(BuildContext context) {
- final ext = Theme.of(context).extension<BeamThemeExtension>()!;
- final isMultiFile = widget.controller.selectedExample?.isMultiFile ?? false;
- final isEnabled = widget.isEditable && !isMultiFile;
-
- return Semantics(
- container: true,
- enabled: isEnabled,
- label: 'widgets.codeEditor.label',
- multiline: true,
- readOnly: isEnabled,
- textField: true,
- child: FocusScope(
- node: FocusScopeNode(canRequestFocus: isEnabled),
- child: CodeTheme(
- data: ext.codeTheme,
- child: Container(
- color: ext.codeTheme.styles['root']?.backgroundColor,
- child: SingleChildScrollView(
- controller: _scrollController,
- child: CodeField(
- key: ValueKey(widget.controller.codeController),
- controller: widget.controller.codeController,
- enabled: isEnabled,
- focusNode: _focusNode,
- textStyle: ext.codeRootStyle,
- ),
- ),
- ),
- ),
- ),
+ return AnimatedBuilder(
+ animation: controller,
+ builder: (context, child) {
+ final count = controller.fileControllers.length;
Review Comment:
`+`
--
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
[GitHub] [beam] Malarg commented on pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
Malarg commented on PR #24865:
URL: https://github.com/apache/beam/pull/24865#issuecomment-1375114274
LGTM
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063210937
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -16,54 +16,33 @@
* limitations under the License.
*/
-import 'dart:math';
-
+import 'package:collection/collection.dart';
import 'package:flutter/widgets.dart';
-import 'package:flutter_code_editor/flutter_code_editor.dart';
-import 'package:get_it/get_it.dart';
import '../models/example.dart';
import '../models/example_loading_descriptors/content_example_loading_descriptor.dart';
import '../models/example_loading_descriptors/empty_example_loading_descriptor.dart';
import '../models/example_loading_descriptors/example_loading_descriptor.dart';
import '../models/example_view_options.dart';
import '../models/sdk.dart';
-import '../services/symbols/symbols_notifier.dart';
+import '../models/snippet_file.dart';
+import 'snippet_file_editing_controller.dart';
/// The main state object for a single [sdk].
class SnippetEditingController extends ChangeNotifier {
+ final List<SnippetFileEditingController> fileControllers = [];
final Sdk sdk;
- final CodeController codeController;
- final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
Example? _selectedExample;
Review Comment:
`+`
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063204472
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -17,12 +17,15 @@
*/
import 'package:flutter/material.dart';
-import 'package:playground/modules/examples/components/description_popover/description_popover_button.dart';
-import 'package:playground/modules/examples/components/multifile_popover/multifile_popover_button.dart';
-import 'package:playground/modules/examples/models/popover_state.dart';
import 'package:playground_components/playground_components.dart';
import 'package:provider/provider.dart';
+import '../../models/popover_state.dart';
+import '../description_popover/description_popover_button.dart';
+import '../multi_file_icon.dart';
+
+const double _iconSize = 30;
Review Comment:
1. `+`
2. This size is for icons to look good in the list, so it is coupled with the list and not with other icons. When we refactor sizing, we will derive it from list item height or text size, but until then let's keep it simple.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063207045
##########
playground/frontend/playground_components/lib/src/controllers/playground_controller.dart:
##########
@@ -400,22 +393,19 @@ class PlaygroundController with ChangeNotifier {
}
Future<UserSharedExampleLoadingDescriptor> saveSnippet() async {
- final controller = requireSnippetEditingController();
- final code = controller.codeController.fullText;
- final name = 'examples.userSharedName'.tr();
+ final snippetController = requireSnippetEditingController();
+ final files = snippetController.getFiles();
final snippetId = await exampleCache.saveSnippet(
- files: [
- SharedFile(code: code, isMain: true, name: name),
- ],
- sdk: controller.sdk,
- pipelineOptions: controller.pipelineOptions,
+ files: files,
Review Comment:
`+`
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063224397
##########
playground/frontend/playground_components/lib/src/repositories/example_client/grpc_example_client.dart:
##########
@@ -364,7 +369,7 @@ class GrpcExampleClient implements ExampleClient {
grpc.SnippetFile()
..name = item.name
..isMain = true
- ..content = item.code,
+ ..content = item.content,
Review Comment:
`+`
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1060651045
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -35,23 +39,14 @@ class ExampleItemActions extends StatelessWidget {
Widget build(BuildContext context) {
return Row(
children: [
- if (example.isMultiFile) multifilePopover,
+ if (example.isMultiFile) const _Icon(MultiFileIcon()),
if (example.complexity != null)
- ComplexityWidget(complexity: example.complexity!),
+ _Icon(ComplexityWidget(complexity: example.complexity!)),
descriptionPopover,
],
);
}
- Widget get multifilePopover => MultifilePopoverButton(
Review Comment:
The popover was to inform that multi-file examples are not runnable. They are runnable now, so we do not need it anymore.
##########
playground/frontend/lib/l10n/app_en.arb:
##########
@@ -195,7 +195,7 @@
"@exampleDescription": {
"description": "Description icon label"
},
- "exampleMultifile": "Multifile example info",
+ "exampleMultifile": "Multifile",
Review Comment:
Because the icon is no longer clickable.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063235740
##########
playground/frontend/playground_components/lib/src/controllers/snippet_file_editing_controller.dart:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'dart:math';
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_code_editor/flutter_code_editor.dart';
+import 'package:get_it/get_it.dart';
+
+import '../models/example_view_options.dart';
+import '../models/sdk.dart';
+import '../models/snippet_file.dart';
+import '../services/symbols/symbols_notifier.dart';
+
+/// The main state object for a file in a snippet.
+class SnippetFileEditingController extends ChangeNotifier {
+ final CodeController codeController;
+ final SnippetFile savedFile;
+ final Sdk sdk;
+
+ bool _isChanged = false;
+
+ final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
+ SnippetFileEditingController({
+ required this.savedFile,
+ required this.sdk,
+ required ExampleViewOptions viewOptions,
+ int? contextLine1Based,
+ }) : codeController = CodeController(
+ language: sdk.highlightMode,
+ namedSectionParser: const BracketsStartEndNamedSectionParser(),
+ text: savedFile.content,
+ ) {
+ _applyViewOptions(viewOptions);
+ if (contextLine1Based != null) {
+ _toStartOfFullLine(max(contextLine1Based - 1, 0));
+ }
+
+ codeController.addListener(_onCodeControllerChanged);
+ _symbolsNotifier.addListener(_onSymbolsNotifierChanged);
+ _onSymbolsNotifierChanged();
+ }
+
+ void _applyViewOptions(ExampleViewOptions options) {
+ codeController.readOnlySectionNames = options.readOnlySectionNames.toSet();
+ codeController.visibleSectionNames = options.showSectionNames.toSet();
+
+ if (options.foldCommentAtLineZero) {
+ codeController.foldCommentAtLineZero();
+ }
+
+ if (options.foldImports) {
+ codeController.foldImports();
+ }
+
+ final unfolded = options.unfoldSectionNames;
+ if (unfolded.isNotEmpty) {
+ codeController.foldOutsideSections(unfolded);
+ }
+ }
+
+ void _toStartOfFullLine(int line) {
+ if (line >= codeController.code.lines.length) {
+ return;
+ }
+
+ final fullPosition = codeController.code.lines.lines[line].textRange.start;
+ final visiblePosition = codeController.code.hiddenRanges.cutPosition(
+ fullPosition,
+ );
+
+ codeController.selection = TextSelection.collapsed(
+ offset: visiblePosition,
+ );
+ }
+
+ void _onCodeControllerChanged() {
+ if (!_isChanged) {
+ if (_isCodeChanged()) {
+ _isChanged = true;
+ notifyListeners();
+ }
+ } else {
+ _updateIsChanged();
+ if (!_isChanged) {
+ notifyListeners();
+ }
+ }
+ }
+
+ bool get isChanged => _isChanged;
+
+ bool _isCodeChanged() {
+ return savedFile.content != codeController.fullText;
+ }
+
+ void _updateIsChanged() {
+ _isChanged = _isCodeChanged();
+ }
+
+ void reset() {
+ codeController.text = savedFile.content;
+ }
+
+ void _onSymbolsNotifierChanged() {
+ final mode = sdk.highlightMode;
+ if (mode == null) {
+ return;
+ }
+
+ final dictionary = _symbolsNotifier.getDictionary(mode);
+ if (dictionary == null) {
+ return;
+ }
+
+ codeController.autocompleter.setCustomWords(dictionary.symbols);
+ }
+
+ SnippetFile getFile() => SnippetFile(
+ content: codeController.fullText,
+ isMain: savedFile.isMain,
+ name: savedFile.name,
+ );
+
+ @override
+ void dispose() {
Review Comment:
I guess `CodeController` and `SnippetFileEditingController` should be deleted together, and the garbage collector should handle the circular references.
Even if it does not now, it is their job for future, and if we micromanage this now we will not gain much but will have more things to maintain.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063237039
##########
playground/frontend/playground_components/lib/src/widgets/snippet_editor.dart:
##########
@@ -16,120 +16,69 @@
* limitations under the License.
*/
-import 'dart:math';
-
import 'package:flutter/material.dart';
-import 'package:flutter/scheduler.dart';
-import 'package:flutter_code_editor/flutter_code_editor.dart';
+import '../constants/sizes.dart';
import '../controllers/snippet_editing_controller.dart';
-import '../theme/theme.dart';
-
-class SnippetEditor extends StatefulWidget {
- final SnippetEditingController controller;
- final bool isEditable;
+import 'loading_indicator.dart';
+import 'snippet_file_editor.dart';
+import 'tabbed_snippet_editor.dart';
- SnippetEditor({
+class SnippetEditor extends StatelessWidget {
+ const SnippetEditor({
required this.controller,
required this.isEditable,
- }) : super(
- // When the example is changed, will scroll to the context line again.
- key: ValueKey(controller.selectedExample),
- );
-
- @override
- State<SnippetEditor> createState() => _SnippetEditorState();
-}
-
-class _SnippetEditorState extends State<SnippetEditor> {
- bool _didAutoFocus = false;
- final _focusNode = FocusNode();
- final _scrollController = ScrollController();
-
- @override
- void didChangeDependencies() {
- super.didChangeDependencies();
-
- if (!_didAutoFocus) {
- _didAutoFocus = true;
- SchedulerBinding.instance.addPostFrameCallback((_) {
- if (mounted) {
- _scrollSoCursorIsOnTop();
- }
- });
- }
- }
-
- void _scrollSoCursorIsOnTop() {
- _focusNode.requestFocus();
+ this.actionsWidget,
+ });
- final position = max(widget.controller.codeController.selection.start, 0);
- final characterOffset = _getLastCharacterOffset(
- text: widget.controller.codeController.text.substring(0, position),
- style: kLightTheme.extension<BeamThemeExtension>()!.codeRootStyle,
- );
+ final SnippetEditingController controller;
Review Comment:
Flutter itself uses just `controller` when the widget makes it clear: `TextField`, `ListView`, etc.
--
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
[GitHub] [beam] Malarg commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
Malarg commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1064271801
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -186,39 +128,84 @@ class SnippetEditingController extends ChangeNotifier {
return ContentExampleLoadingDescriptor(
complexity: example.complexity,
- content: codeController.fullText,
+ files: getFiles(),
name: example.name,
sdk: sdk,
);
}
- void setSource(String source) {
- codeController.readOnlySectionNames = const {};
- codeController.visibleSectionNames = const {};
+ void _replaceFileControllers(
+ Iterable<SnippetFile> files,
+ ExampleViewOptions viewOptions,
+ ) {
+ for (final oldController in fileControllers) {
+ oldController.removeListener(_onFileControllerChanged);
+ }
+ final newControllers = <SnippetFileEditingController>[];
+
+ for (final file in files) {
+ final controller = SnippetFileEditingController(
+ contextLine1Based: file.isMain ? _selectedExample?.contextLine : null,
+ savedFile: file,
+ sdk: sdk,
+ viewOptions: viewOptions,
+ );
+
+ newControllers.add(controller);
+ controller.addListener(_onFileControllerChanged);
+ }
- codeController.fullText = source;
- codeController.historyController.deleteHistory();
- }
+ for (final oldController in fileControllers) {
+ oldController.dispose();
+ }
- void _onSymbolsNotifierChanged() {
- final mode = sdk.highlightMode;
- if (mode == null) {
- return;
+ fileControllers.clear();
+ fileControllers.addAll(newControllers);
+
+ _fileControllersByName.clear();
+ for (final controller in newControllers) {
+ _fileControllersByName[controller.savedFile.name] = controller;
}
- final dictionary = _symbolsNotifier.getDictionary(mode);
- if (dictionary == null) {
- return;
+ _activeFileController =
+ fileControllers.firstWhereOrNull((c) => c.savedFile.isMain);
+ }
+
+ void _onFileControllerChanged() {
+ if (!_isChanged) {
+ if (_isAnyFileControllerChanged()) {
+ _isChanged = true;
+ notifyListeners();
+ }
+ } else {
+ _updateIsChanged();
+ if (!_isChanged) {
+ notifyListeners();
+ }
}
+ }
- codeController.autocompleter.setCustomWords(dictionary.symbols);
+ bool _isAnyFileControllerChanged() {
Review Comment:
Oh, got the naming. Don't mind :)
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063229787
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -16,54 +16,33 @@
* limitations under the License.
*/
-import 'dart:math';
-
+import 'package:collection/collection.dart';
import 'package:flutter/widgets.dart';
-import 'package:flutter_code_editor/flutter_code_editor.dart';
-import 'package:get_it/get_it.dart';
import '../models/example.dart';
import '../models/example_loading_descriptors/content_example_loading_descriptor.dart';
import '../models/example_loading_descriptors/empty_example_loading_descriptor.dart';
import '../models/example_loading_descriptors/example_loading_descriptor.dart';
import '../models/example_view_options.dart';
import '../models/sdk.dart';
-import '../services/symbols/symbols_notifier.dart';
+import '../models/snippet_file.dart';
+import 'snippet_file_editing_controller.dart';
/// The main state object for a single [sdk].
class SnippetEditingController extends ChangeNotifier {
+ final List<SnippetFileEditingController> fileControllers = [];
Review Comment:
`+`
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063227148
##########
playground/frontend/playground_components/lib/src/cache/example_cache.dart:
##########
@@ -170,9 +170,9 @@ class ExampleCache extends ChangeNotifier {
return Example.fromBase(
example,
- source: exampleData[0],
- outputs: exampleData[1],
- logs: exampleData[2],
+ files: exampleData[0] as List<SnippetFile>,
Review Comment:
- Will be addressed in https://github.com/apache/beam/issues/24305
--
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
[GitHub] [beam] nausharipov commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
nausharipov commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1061403825
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -65,3 +59,21 @@ class ExampleItemActions extends StatelessWidget {
Provider.of<PopoverState>(context, listen: false).setOpen(isOpen);
}
}
+
+/// A wrapper of a standard size for icons in the example list.
+class _Icon extends StatelessWidget {
+ const _Icon(this.child);
+
+ final Widget child;
+
+ @override
+ Widget build(BuildContext context) {
+ return SizedBox(
+ width: _iconSize,
Review Comment:
sort `height` and `width`.
##########
playground/frontend/playground_components/lib/src/controllers/playground_controller.dart:
##########
@@ -400,22 +393,19 @@ class PlaygroundController with ChangeNotifier {
}
Future<UserSharedExampleLoadingDescriptor> saveSnippet() async {
- final controller = requireSnippetEditingController();
- final code = controller.codeController.fullText;
- final name = 'examples.userSharedName'.tr();
+ final snippetController = requireSnippetEditingController();
+ final files = snippetController.getFiles();
final snippetId = await exampleCache.saveSnippet(
- files: [
- SharedFile(code: code, isMain: true, name: name),
- ],
- sdk: controller.sdk,
- pipelineOptions: controller.pipelineOptions,
+ files: files,
+ sdk: snippetController.sdk,
+ pipelineOptions: snippetController.pipelineOptions,
);
final sharedExample = Example(
- source: code,
- name: name,
- sdk: controller.sdk,
+ files: files,
+ name: files.first.name,
Review Comment:
Can `files` be empty?
##########
playground/frontend/playground_components/lib/src/controllers/snippet_file_editing_controller.dart:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'dart:math';
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_code_editor/flutter_code_editor.dart';
+import 'package:get_it/get_it.dart';
+
+import '../models/example_view_options.dart';
+import '../models/sdk.dart';
+import '../models/snippet_file.dart';
+import '../services/symbols/symbols_notifier.dart';
+
+/// The main state object for a file in a snippet.
+class SnippetFileEditingController extends ChangeNotifier {
+ final CodeController codeController;
+ final SnippetFile savedFile;
+ final Sdk sdk;
+
+ bool _isChanged = false;
+
+ final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
+ SnippetFileEditingController({
+ required this.savedFile,
+ required this.sdk,
+ required ExampleViewOptions viewOptions,
+ int? contextLine1Based,
+ }) : codeController = CodeController(
+ language: sdk.highlightMode,
+ namedSectionParser: const BracketsStartEndNamedSectionParser(),
+ text: savedFile.content,
+ ) {
+ _applyViewOptions(viewOptions);
+ if (contextLine1Based != null) {
+ _toStartOfFullLine(max(contextLine1Based - 1, 0));
+ }
+
+ codeController.addListener(_onCodeControllerChanged);
+ _symbolsNotifier.addListener(_onSymbolsNotifierChanged);
+ _onSymbolsNotifierChanged();
+ }
+
+ void _applyViewOptions(ExampleViewOptions options) {
+ codeController.readOnlySectionNames = options.readOnlySectionNames.toSet();
+ codeController.visibleSectionNames = options.showSectionNames.toSet();
+
+ if (options.foldCommentAtLineZero) {
+ codeController.foldCommentAtLineZero();
+ }
+
+ if (options.foldImports) {
+ codeController.foldImports();
+ }
+
+ final unfolded = options.unfoldSectionNames;
+ if (unfolded.isNotEmpty) {
+ codeController.foldOutsideSections(unfolded);
+ }
+ }
+
+ void _toStartOfFullLine(int line) {
+ if (line >= codeController.code.lines.length) {
+ return;
+ }
+
+ final fullPosition = codeController.code.lines.lines[line].textRange.start;
+ final visiblePosition = codeController.code.hiddenRanges.cutPosition(
+ fullPosition,
+ );
+
+ codeController.selection = TextSelection.collapsed(
+ offset: visiblePosition,
+ );
+ }
+
+ void _onCodeControllerChanged() {
+ if (!_isChanged) {
+ if (_isCodeChanged()) {
+ _isChanged = true;
+ notifyListeners();
+ }
+ } else {
+ _updateIsChanged();
+ if (!_isChanged) {
+ notifyListeners();
+ }
+ }
+ }
+
+ bool get isChanged => _isChanged;
+
+ bool _isCodeChanged() {
+ return savedFile.content != codeController.fullText;
+ }
+
+ void _updateIsChanged() {
+ _isChanged = _isCodeChanged();
+ }
+
+ void reset() {
+ codeController.text = savedFile.content;
+ }
+
+ void _onSymbolsNotifierChanged() {
+ final mode = sdk.highlightMode;
+ if (mode == null) {
+ return;
+ }
+
+ final dictionary = _symbolsNotifier.getDictionary(mode);
+ if (dictionary == null) {
+ return;
+ }
+
+ codeController.autocompleter.setCustomWords(dictionary.symbols);
+ }
+
+ SnippetFile getFile() => SnippetFile(
+ content: codeController.fullText,
+ isMain: savedFile.isMain,
+ name: savedFile.name,
+ );
+
+ @override
+ void dispose() {
Review Comment:
Seems like `codeController` listener is not removed.
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -156,19 +85,32 @@ class SnippetEditingController extends ChangeNotifier {
bool get isChanged => _isChanged;
void _updateIsChanged() {
- _isChanged = _isCodeChanged() || _arePipelineOptionsChanged();
+ _isChanged = _calculateIsChanged();
}
- bool _isCodeChanged() {
- return _selectedExample?.source != codeController.fullText;
+ bool _calculateIsChanged() {
+ for (final controller in fileControllers) {
Review Comment:
Rename `controller` to `fileController`.
##########
playground/frontend/playground_components/lib/src/widgets/snippet_editor.dart:
##########
@@ -16,120 +16,69 @@
* limitations under the License.
*/
-import 'dart:math';
-
import 'package:flutter/material.dart';
-import 'package:flutter/scheduler.dart';
-import 'package:flutter_code_editor/flutter_code_editor.dart';
+import '../constants/sizes.dart';
import '../controllers/snippet_editing_controller.dart';
-import '../theme/theme.dart';
-
-class SnippetEditor extends StatefulWidget {
- final SnippetEditingController controller;
- final bool isEditable;
+import 'loading_indicator.dart';
+import 'snippet_file_editor.dart';
+import 'tabbed_snippet_editor.dart';
- SnippetEditor({
+class SnippetEditor extends StatelessWidget {
+ const SnippetEditor({
required this.controller,
required this.isEditable,
- }) : super(
- // When the example is changed, will scroll to the context line again.
- key: ValueKey(controller.selectedExample),
- );
-
- @override
- State<SnippetEditor> createState() => _SnippetEditorState();
-}
-
-class _SnippetEditorState extends State<SnippetEditor> {
- bool _didAutoFocus = false;
- final _focusNode = FocusNode();
- final _scrollController = ScrollController();
-
- @override
- void didChangeDependencies() {
- super.didChangeDependencies();
-
- if (!_didAutoFocus) {
- _didAutoFocus = true;
- SchedulerBinding.instance.addPostFrameCallback((_) {
- if (mounted) {
- _scrollSoCursorIsOnTop();
- }
- });
- }
- }
-
- void _scrollSoCursorIsOnTop() {
- _focusNode.requestFocus();
+ this.actionsWidget,
+ });
- final position = max(widget.controller.codeController.selection.start, 0);
- final characterOffset = _getLastCharacterOffset(
- text: widget.controller.codeController.text.substring(0, position),
- style: kLightTheme.extension<BeamThemeExtension>()!.codeRootStyle,
- );
+ final SnippetEditingController controller;
+ final bool isEditable;
- _scrollController.jumpTo(
- min(
- characterOffset.dy,
- _scrollController.position.maxScrollExtent,
- ),
- );
- }
-
- @override
- void dispose() {
- _focusNode.dispose();
- super.dispose();
- }
+ /// A child widget that will be:
+ /// - Hidden if no file is loaded.
+ /// - Shown as an overlay for a single file editor.
+ /// - Built into the tab bar for a multi-file editor.
+ final Widget? actionsWidget;
@override
Widget build(BuildContext context) {
- final ext = Theme.of(context).extension<BeamThemeExtension>()!;
- final isMultiFile = widget.controller.selectedExample?.isMultiFile ?? false;
- final isEnabled = widget.isEditable && !isMultiFile;
-
- return Semantics(
- container: true,
- enabled: isEnabled,
- label: 'widgets.codeEditor.label',
- multiline: true,
- readOnly: isEnabled,
- textField: true,
- child: FocusScope(
- node: FocusScopeNode(canRequestFocus: isEnabled),
- child: CodeTheme(
- data: ext.codeTheme,
- child: Container(
- color: ext.codeTheme.styles['root']?.backgroundColor,
- child: SingleChildScrollView(
- controller: _scrollController,
- child: CodeField(
- key: ValueKey(widget.controller.codeController),
- controller: widget.controller.codeController,
- enabled: isEnabled,
- focusNode: _focusNode,
- textStyle: ext.codeRootStyle,
- ),
- ),
- ),
- ),
- ),
+ return AnimatedBuilder(
+ animation: controller,
+ builder: (context, child) {
+ final count = controller.fileControllers.length;
Review Comment:
`filesCount`?
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -16,54 +16,33 @@
* limitations under the License.
*/
-import 'dart:math';
-
+import 'package:collection/collection.dart';
import 'package:flutter/widgets.dart';
-import 'package:flutter_code_editor/flutter_code_editor.dart';
-import 'package:get_it/get_it.dart';
import '../models/example.dart';
import '../models/example_loading_descriptors/content_example_loading_descriptor.dart';
import '../models/example_loading_descriptors/empty_example_loading_descriptor.dart';
import '../models/example_loading_descriptors/example_loading_descriptor.dart';
import '../models/example_view_options.dart';
import '../models/sdk.dart';
-import '../services/symbols/symbols_notifier.dart';
+import '../models/snippet_file.dart';
+import 'snippet_file_editing_controller.dart';
/// The main state object for a single [sdk].
class SnippetEditingController extends ChangeNotifier {
+ final List<SnippetFileEditingController> fileControllers = [];
Review Comment:
Why isn't `fileControllers` private with a getter?
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -35,23 +38,14 @@ class ExampleItemActions extends StatelessWidget {
Widget build(BuildContext context) {
return Row(
children: [
- if (example.isMultiFile) multifilePopover,
+ if (example.isMultiFile) const _Icon(MultiFileIcon()),
Review Comment:
Why not to pass `_iconSize` as an argument to `MultiFileIcon` instead of using a wrapper widget?
##########
playground/frontend/lib/pages/standalone_playground/widgets/editor_textarea_wrapper.dart:
##########
@@ -47,60 +46,34 @@ class CodeTextAreaWrapper extends StatelessWidget {
return const LoadingIndicator();
}
- return Column(
- children: [
- Expanded(
- child: Stack(
- children: [
- Positioned.fill(
- child: SnippetEditor(
- controller: snippetController,
- isEditable: true,
- ),
+ return SnippetEditor(
+ controller: snippetController,
+ isEditable: true,
+ actionsWidget: Row(
+ children: [
+ if (controller.selectedExample != null)
+ Semantics(
+ container: true,
+ child: DescriptionPopoverButton(
+ example: controller.selectedExample!,
Review Comment:
A variable for `controller.selectedExample` would make `!` unnecessary.
##########
playground/frontend/playground_components/lib/src/controllers/snippet_file_editing_controller.dart:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'dart:math';
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_code_editor/flutter_code_editor.dart';
+import 'package:get_it/get_it.dart';
+
+import '../models/example_view_options.dart';
+import '../models/sdk.dart';
+import '../models/snippet_file.dart';
+import '../services/symbols/symbols_notifier.dart';
+
+/// The main state object for a file in a snippet.
+class SnippetFileEditingController extends ChangeNotifier {
+ final CodeController codeController;
+ final SnippetFile savedFile;
+ final Sdk sdk;
+
+ bool _isChanged = false;
+
+ final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
+ SnippetFileEditingController({
+ required this.savedFile,
+ required this.sdk,
+ required ExampleViewOptions viewOptions,
+ int? contextLine1Based,
+ }) : codeController = CodeController(
+ language: sdk.highlightMode,
+ namedSectionParser: const BracketsStartEndNamedSectionParser(),
+ text: savedFile.content,
+ ) {
+ _applyViewOptions(viewOptions);
+ if (contextLine1Based != null) {
+ _toStartOfFullLine(max(contextLine1Based - 1, 0));
+ }
+
+ codeController.addListener(_onCodeControllerChanged);
+ _symbolsNotifier.addListener(_onSymbolsNotifierChanged);
+ _onSymbolsNotifierChanged();
+ }
+
+ void _applyViewOptions(ExampleViewOptions options) {
+ codeController.readOnlySectionNames = options.readOnlySectionNames.toSet();
+ codeController.visibleSectionNames = options.showSectionNames.toSet();
+
+ if (options.foldCommentAtLineZero) {
+ codeController.foldCommentAtLineZero();
+ }
+
+ if (options.foldImports) {
+ codeController.foldImports();
+ }
+
+ final unfolded = options.unfoldSectionNames;
+ if (unfolded.isNotEmpty) {
+ codeController.foldOutsideSections(unfolded);
+ }
+ }
+
+ void _toStartOfFullLine(int line) {
Review Comment:
`_toStart**From**FullLine`?
##########
playground/frontend/playground_components/lib/src/controllers/snippet_file_editing_controller.dart:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'dart:math';
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_code_editor/flutter_code_editor.dart';
+import 'package:get_it/get_it.dart';
+
+import '../models/example_view_options.dart';
+import '../models/sdk.dart';
+import '../models/snippet_file.dart';
+import '../services/symbols/symbols_notifier.dart';
+
+/// The main state object for a file in a snippet.
+class SnippetFileEditingController extends ChangeNotifier {
+ final CodeController codeController;
+ final SnippetFile savedFile;
+ final Sdk sdk;
+
+ bool _isChanged = false;
+
+ final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
+ SnippetFileEditingController({
+ required this.savedFile,
+ required this.sdk,
+ required ExampleViewOptions viewOptions,
+ int? contextLine1Based,
+ }) : codeController = CodeController(
+ language: sdk.highlightMode,
+ namedSectionParser: const BracketsStartEndNamedSectionParser(),
+ text: savedFile.content,
+ ) {
+ _applyViewOptions(viewOptions);
+ if (contextLine1Based != null) {
+ _toStartOfFullLine(max(contextLine1Based - 1, 0));
+ }
+
+ codeController.addListener(_onCodeControllerChanged);
+ _symbolsNotifier.addListener(_onSymbolsNotifierChanged);
+ _onSymbolsNotifierChanged();
+ }
+
+ void _applyViewOptions(ExampleViewOptions options) {
+ codeController.readOnlySectionNames = options.readOnlySectionNames.toSet();
+ codeController.visibleSectionNames = options.showSectionNames.toSet();
+
+ if (options.foldCommentAtLineZero) {
+ codeController.foldCommentAtLineZero();
+ }
+
+ if (options.foldImports) {
+ codeController.foldImports();
+ }
+
+ final unfolded = options.unfoldSectionNames;
+ if (unfolded.isNotEmpty) {
+ codeController.foldOutsideSections(unfolded);
+ }
+ }
+
+ void _toStartOfFullLine(int line) {
+ if (line >= codeController.code.lines.length) {
+ return;
+ }
+
+ final fullPosition = codeController.code.lines.lines[line].textRange.start;
+ final visiblePosition = codeController.code.hiddenRanges.cutPosition(
+ fullPosition,
+ );
+
+ codeController.selection = TextSelection.collapsed(
+ offset: visiblePosition,
+ );
+ }
+
+ void _onCodeControllerChanged() {
+ if (!_isChanged) {
Review Comment:
`_isFileChanged`?
##########
playground/frontend/playground_components/lib/src/cache/example_cache.dart:
##########
@@ -170,9 +170,9 @@ class ExampleCache extends ChangeNotifier {
return Example.fromBase(
example,
- source: exampleData[0],
- outputs: exampleData[1],
- logs: exampleData[2],
+ files: exampleData[0] as List<SnippetFile>,
Review Comment:
0, 1, 2, 3 seem like magic numbers.
##########
playground/frontend/lib/pages/standalone_playground/widgets/editor_textarea_wrapper.dart:
##########
@@ -47,60 +46,34 @@ class CodeTextAreaWrapper extends StatelessWidget {
return const LoadingIndicator();
}
- return Column(
- children: [
- Expanded(
- child: Stack(
- children: [
- Positioned.fill(
- child: SnippetEditor(
- controller: snippetController,
- isEditable: true,
- ),
+ return SnippetEditor(
+ controller: snippetController,
+ isEditable: true,
+ actionsWidget: Row(
+ children: [
+ if (controller.selectedExample != null)
+ Semantics(
+ container: true,
+ child: DescriptionPopoverButton(
+ example: controller.selectedExample!,
+ followerAnchor: Alignment.topRight,
+ targetAnchor: Alignment.bottomRight,
),
- Positioned(
- right: kXlSpacing,
- top: kXlSpacing,
- height: kButtonHeight,
- child: Row(
- children: [
- if (controller.selectedExample != null) ...[
- if (controller.selectedExample?.isMultiFile ?? false)
- Semantics(
- container: true,
- child: MultifilePopoverButton(
- example: controller.selectedExample!,
- followerAnchor: Alignment.topRight,
- targetAnchor: Alignment.bottomRight,
- ),
- ),
- Semantics(
- container: true,
- child: DescriptionPopoverButton(
- example: controller.selectedExample!,
- followerAnchor: Alignment.topRight,
- targetAnchor: Alignment.bottomRight,
- ),
- ),
- ],
- Semantics(
- container: true,
- child: ShareButton(
- playgroundController: controller,
- ),
- ),
- const SizedBox(width: kLgSpacing),
- Semantics(
- container: true,
- child: const PlaygroundRunOrCancelButton(),
- ),
- ],
- ),
- ),
- ],
+ ),
+ Semantics(
+ container: true,
+ child: ShareButton(
+ playgroundController: controller,
Review Comment:
Rename `controller` to `playgroundController`.
##########
playground/frontend/playground_components/lib/src/models/example_loading_descriptors/content_example_loading_descriptor.dart:
##########
@@ -53,7 +53,9 @@ class ContentExampleLoadingDescriptor extends ExampleLoadingDescriptor {
}
return ContentExampleLoadingDescriptor(
- content: content,
+ files: (map['files'] as List<dynamic>)
+ .map((e) => SnippetFile.fromJson(e as Map<String, dynamic>))
Review Comment:
`e` => `file`
##########
playground/frontend/playground_components/lib/src/widgets/snippet_editor.dart:
##########
@@ -16,120 +16,69 @@
* limitations under the License.
*/
-import 'dart:math';
-
import 'package:flutter/material.dart';
-import 'package:flutter/scheduler.dart';
-import 'package:flutter_code_editor/flutter_code_editor.dart';
+import '../constants/sizes.dart';
import '../controllers/snippet_editing_controller.dart';
-import '../theme/theme.dart';
-
-class SnippetEditor extends StatefulWidget {
- final SnippetEditingController controller;
- final bool isEditable;
+import 'loading_indicator.dart';
+import 'snippet_file_editor.dart';
+import 'tabbed_snippet_editor.dart';
- SnippetEditor({
+class SnippetEditor extends StatelessWidget {
+ const SnippetEditor({
required this.controller,
required this.isEditable,
- }) : super(
- // When the example is changed, will scroll to the context line again.
- key: ValueKey(controller.selectedExample),
- );
-
- @override
- State<SnippetEditor> createState() => _SnippetEditorState();
-}
-
-class _SnippetEditorState extends State<SnippetEditor> {
- bool _didAutoFocus = false;
- final _focusNode = FocusNode();
- final _scrollController = ScrollController();
-
- @override
- void didChangeDependencies() {
- super.didChangeDependencies();
-
- if (!_didAutoFocus) {
- _didAutoFocus = true;
- SchedulerBinding.instance.addPostFrameCallback((_) {
- if (mounted) {
- _scrollSoCursorIsOnTop();
- }
- });
- }
- }
-
- void _scrollSoCursorIsOnTop() {
- _focusNode.requestFocus();
+ this.actionsWidget,
+ });
- final position = max(widget.controller.codeController.selection.start, 0);
- final characterOffset = _getLastCharacterOffset(
- text: widget.controller.codeController.text.substring(0, position),
- style: kLightTheme.extension<BeamThemeExtension>()!.codeRootStyle,
- );
+ final SnippetEditingController controller;
Review Comment:
`snippetEditingController`?
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -186,39 +128,84 @@ class SnippetEditingController extends ChangeNotifier {
return ContentExampleLoadingDescriptor(
complexity: example.complexity,
- content: codeController.fullText,
+ files: getFiles(),
name: example.name,
sdk: sdk,
);
}
- void setSource(String source) {
- codeController.readOnlySectionNames = const {};
- codeController.visibleSectionNames = const {};
+ void _replaceFileControllers(
Review Comment:
`_updateFileControllers` is more clear to me than `_replaceFileControllers`.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063206466
##########
playground/frontend/playground_components/lib/src/controllers/example_loaders/http_example_loader.dart:
##########
@@ -48,9 +49,11 @@ class HttpExampleLoader extends ExampleLoader {
return Example(
name: descriptor.uri.path.split('/').lastOrNull ?? 'HTTP Example',
+ files: [
Review Comment:
HTTP loader was an ad-hoc thing to load examples for the demo but is neat enough to keep it. We have no production use cases for it now, so no need to add functionality to it.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063220026
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -186,39 +128,84 @@ class SnippetEditingController extends ChangeNotifier {
return ContentExampleLoadingDescriptor(
complexity: example.complexity,
- content: codeController.fullText,
+ files: getFiles(),
name: example.name,
sdk: sdk,
);
}
- void setSource(String source) {
- codeController.readOnlySectionNames = const {};
- codeController.visibleSectionNames = const {};
+ void _replaceFileControllers(
Review Comment:
`+`
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063228347
##########
playground/frontend/lib/pages/standalone_playground/widgets/editor_textarea_wrapper.dart:
##########
@@ -47,60 +46,34 @@ class CodeTextAreaWrapper extends StatelessWidget {
return const LoadingIndicator();
}
- return Column(
- children: [
- Expanded(
- child: Stack(
- children: [
- Positioned.fill(
- child: SnippetEditor(
- controller: snippetController,
- isEditable: true,
- ),
+ return SnippetEditor(
+ controller: snippetController,
+ isEditable: true,
+ actionsWidget: Row(
+ children: [
+ if (controller.selectedExample != null)
+ Semantics(
+ container: true,
+ child: DescriptionPopoverButton(
+ example: controller.selectedExample!,
+ followerAnchor: Alignment.topRight,
+ targetAnchor: Alignment.bottomRight,
),
- Positioned(
- right: kXlSpacing,
- top: kXlSpacing,
- height: kButtonHeight,
- child: Row(
- children: [
- if (controller.selectedExample != null) ...[
- if (controller.selectedExample?.isMultiFile ?? false)
- Semantics(
- container: true,
- child: MultifilePopoverButton(
- example: controller.selectedExample!,
- followerAnchor: Alignment.topRight,
- targetAnchor: Alignment.bottomRight,
- ),
- ),
- Semantics(
- container: true,
- child: DescriptionPopoverButton(
- example: controller.selectedExample!,
- followerAnchor: Alignment.topRight,
- targetAnchor: Alignment.bottomRight,
- ),
- ),
- ],
- Semantics(
- container: true,
- child: ShareButton(
- playgroundController: controller,
- ),
- ),
- const SizedBox(width: kLgSpacing),
- Semantics(
- container: true,
- child: const PlaygroundRunOrCancelButton(),
- ),
- ],
- ),
- ),
- ],
+ ),
+ Semantics(
+ container: true,
+ child: ShareButton(
+ playgroundController: controller,
Review Comment:
`+`
--
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
[GitHub] [beam] github-actions[bot] commented on pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24865:
URL: https://github.com/apache/beam/pull/24865#issuecomment-1375736739
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
--
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
[GitHub] [beam] alexeyinkin commented on pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on PR #24865:
URL: https://github.com/apache/beam/pull/24865#issuecomment-1375735116
R: @damondouglas
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063221708
##########
playground/frontend/playground_components/lib/src/controllers/snippet_editing_controller.dart:
##########
@@ -186,39 +128,84 @@ class SnippetEditingController extends ChangeNotifier {
return ContentExampleLoadingDescriptor(
complexity: example.complexity,
- content: codeController.fullText,
+ files: getFiles(),
name: example.name,
sdk: sdk,
);
}
- void setSource(String source) {
- codeController.readOnlySectionNames = const {};
- codeController.visibleSectionNames = const {};
+ void _replaceFileControllers(
+ Iterable<SnippetFile> files,
+ ExampleViewOptions viewOptions,
+ ) {
+ for (final oldController in fileControllers) {
+ oldController.removeListener(_onFileControllerChanged);
+ }
+ final newControllers = <SnippetFileEditingController>[];
+
+ for (final file in files) {
+ final controller = SnippetFileEditingController(
+ contextLine1Based: file.isMain ? _selectedExample?.contextLine : null,
+ savedFile: file,
+ sdk: sdk,
+ viewOptions: viewOptions,
+ );
+
+ newControllers.add(controller);
+ controller.addListener(_onFileControllerChanged);
+ }
- codeController.fullText = source;
- codeController.historyController.deleteHistory();
- }
+ for (final oldController in fileControllers) {
+ oldController.dispose();
+ }
- void _onSymbolsNotifierChanged() {
- final mode = sdk.highlightMode;
- if (mode == null) {
- return;
+ fileControllers.clear();
+ fileControllers.addAll(newControllers);
+
+ _fileControllersByName.clear();
+ for (final controller in newControllers) {
+ _fileControllersByName[controller.savedFile.name] = controller;
}
- final dictionary = _symbolsNotifier.getDictionary(mode);
- if (dictionary == null) {
- return;
+ _activeFileController =
+ fileControllers.firstWhereOrNull((c) => c.savedFile.isMain);
+ }
+
+ void _onFileControllerChanged() {
+ if (!_isChanged) {
+ if (_isAnyFileControllerChanged()) {
+ _isChanged = true;
+ notifyListeners();
+ }
+ } else {
+ _updateIsChanged();
+ if (!_isChanged) {
+ notifyListeners();
+ }
}
+ }
- codeController.autocompleter.setCustomWords(dictionary.symbols);
+ bool _isAnyFileControllerChanged() {
+ return fileControllers.any((c) => c.isChanged);
}
- @override
- void dispose() {
- _symbolsNotifier.removeListener(
- _onSymbolsNotifierChanged,
- );
- super.dispose();
+ SnippetFileEditingController? get activeFileController =>
+ _activeFileController;
+
+ SnippetFileEditingController? getFileControllerByName(String name) {
+ return _fileControllersByName[name];
+ }
+
+ void activateFileControllerByName(String name) {
+ final newController = getFileControllerByName(name);
Review Comment:
Yes, this case is valid and handled. Added a test.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063224919
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -35,23 +38,14 @@ class ExampleItemActions extends StatelessWidget {
Widget build(BuildContext context) {
return Row(
children: [
- if (example.isMultiFile) multifilePopover,
+ if (example.isMultiFile) const _Icon(MultiFileIcon()),
Review Comment:
Moved into the class.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063205621
##########
playground/frontend/lib/pages/standalone_playground/widgets/editor_textarea_wrapper.dart:
##########
@@ -47,60 +46,34 @@ class CodeTextAreaWrapper extends StatelessWidget {
return const LoadingIndicator();
}
- return Column(
- children: [
- Expanded(
- child: Stack(
- children: [
- Positioned.fill(
- child: SnippetEditor(
- controller: snippetController,
- isEditable: true,
- ),
+ return SnippetEditor(
+ controller: snippetController,
+ isEditable: true,
+ actionsWidget: Row(
Review Comment:
This whole file should be refactored later.
--
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
[GitHub] [beam] alexeyinkin commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063204853
##########
playground/frontend/lib/modules/examples/components/example_list/example_item_actions.dart:
##########
@@ -65,3 +59,21 @@ class ExampleItemActions extends StatelessWidget {
Provider.of<PopoverState>(context, listen: false).setOpen(isOpen);
}
}
+
+/// A wrapper of a standard size for icons in the example list.
+class _Icon extends StatelessWidget {
+ const _Icon(this.child);
Review Comment:
To a private class with a single argument this is overengineering.
--
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
[GitHub] [beam] nausharipov commented on a diff in pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
nausharipov commented on code in PR #24865:
URL: https://github.com/apache/beam/pull/24865#discussion_r1063244144
##########
playground/frontend/playground_components/lib/src/controllers/snippet_file_editing_controller.dart:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import 'dart:math';
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_code_editor/flutter_code_editor.dart';
+import 'package:get_it/get_it.dart';
+
+import '../models/example_view_options.dart';
+import '../models/sdk.dart';
+import '../models/snippet_file.dart';
+import '../services/symbols/symbols_notifier.dart';
+
+/// The main state object for a file in a snippet.
+class SnippetFileEditingController extends ChangeNotifier {
+ final CodeController codeController;
+ final SnippetFile savedFile;
+ final Sdk sdk;
+
+ bool _isChanged = false;
+
+ final _symbolsNotifier = GetIt.instance.get<SymbolsNotifier>();
+
+ SnippetFileEditingController({
+ required this.savedFile,
+ required this.sdk,
+ required ExampleViewOptions viewOptions,
+ int? contextLine1Based,
+ }) : codeController = CodeController(
+ language: sdk.highlightMode,
+ namedSectionParser: const BracketsStartEndNamedSectionParser(),
+ text: savedFile.content,
+ ) {
+ _applyViewOptions(viewOptions);
+ if (contextLine1Based != null) {
+ _toStartOfFullLine(max(contextLine1Based - 1, 0));
+ }
+
+ codeController.addListener(_onCodeControllerChanged);
+ _symbolsNotifier.addListener(_onSymbolsNotifierChanged);
+ _onSymbolsNotifierChanged();
+ }
+
+ void _applyViewOptions(ExampleViewOptions options) {
+ codeController.readOnlySectionNames = options.readOnlySectionNames.toSet();
+ codeController.visibleSectionNames = options.showSectionNames.toSet();
+
+ if (options.foldCommentAtLineZero) {
+ codeController.foldCommentAtLineZero();
+ }
+
+ if (options.foldImports) {
+ codeController.foldImports();
+ }
+
+ final unfolded = options.unfoldSectionNames;
+ if (unfolded.isNotEmpty) {
+ codeController.foldOutsideSections(unfolded);
+ }
+ }
+
+ void _toStartOfFullLine(int line) {
Review Comment:
Ok, I didn't understand the meaning initially.
--
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
[GitHub] [beam] pabloem merged pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
pabloem merged PR #24865:
URL: https://github.com/apache/beam/pull/24865
--
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
[GitHub] [beam] pabloem commented on pull request #24865: Multifile examples on frontend (#24859)
Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #24865:
URL: https://github.com/apache/beam/pull/24865#issuecomment-1379368358
lgtm thanks y'all!
--
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