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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
