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 2021/10/14 15:04:18 UTC
[GitHub] [beam] damondouglas commented on a change in pull request #15709: [BEAM-12967] [Playground] Implemented example selector
damondouglas commented on a change in pull request #15709:
URL: https://github.com/apache/beam/pull/15709#discussion_r729068827
##########
File path: playground/frontend/lib/modules/examples/components/filter/category_bubble.dart
##########
@@ -0,0 +1,65 @@
+/*
+ * 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 'package:flutter/material.dart';
+import 'package:playground/config/theme.dart';
+import 'package:playground/constants/sizes.dart';
+import 'package:playground/pages/playground/states/example_dropdown_state.dart';
+import 'package:provider/provider.dart';
+
+class CategoryBubble extends StatelessWidget {
+ final String name;
+
+ const CategoryBubble({Key? key, required this.name}) : super(key: key);
+
+ @override
+ Widget build(BuildContext context) {
+ return Padding(
+ padding: const EdgeInsets.only(right: kMdSpacing),
+ child: Consumer<ExampleDropdownState>(
+ builder: (context, state, child) => GestureDetector(
Review comment:
May we wrap this `GestureDetector` widget with a `MouseRegion` as its parent and assign the `cursor` property of the `MouseRegion` parent widget to `SystemMouseCursors.click`. The outcome of this change will be on mouse hover, the UI will communicate to the user that the widget is clickable.
##########
File path: playground/frontend/lib/modules/examples/components/filter/category_bubble.dart
##########
@@ -0,0 +1,65 @@
+/*
+ * 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 'package:flutter/material.dart';
+import 'package:playground/config/theme.dart';
+import 'package:playground/constants/sizes.dart';
+import 'package:playground/pages/playground/states/example_dropdown_state.dart';
+import 'package:provider/provider.dart';
+
+class CategoryBubble extends StatelessWidget {
+ final String name;
+
+ const CategoryBubble({Key? key, required this.name}) : super(key: key);
+
+ @override
+ Widget build(BuildContext context) {
+ return Padding(
+ padding: const EdgeInsets.only(right: kMdSpacing),
+ child: Consumer<ExampleDropdownState>(
+ builder: (context, state, child) => GestureDetector(
+ onTap: () {
+ if (name != state.selectedCategory) {
+ state.setSelectedCategory(name);
+ }
+ },
+ child: Container(
+ height: kContainerHeight,
Review comment:
This comment is **not** blocking as it is subjective. I notice that the height for the All, Examples, Katas and Unit tests seem a little too high. It's more noticeable with the All button. I took the font size 14px and multiplied by the golden ratio 1.618 and replaced `kContainerHeight` with this `22.652` which seemed to show a more balanced negative space padding. Again, this is subjective.
##########
File path: playground/frontend/lib/modules/examples/components/example_list/expansion_panel_item.dart
##########
@@ -0,0 +1,53 @@
+/*
+ * 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 'package:flutter/material.dart';
+import 'package:playground/constants/sizes.dart';
+import 'package:playground/modules/examples/models/example_model.dart';
+import 'package:playground/pages/playground/states/playground_state.dart';
+import 'package:provider/provider.dart';
+
+class ExpansionPanelItem extends StatelessWidget {
+ final ExampleModel example;
+
+ const ExpansionPanelItem({Key? key, required this.example}) : super(key: key);
+
+ @override
+ Widget build(BuildContext context) {
+ return Consumer<PlaygroundState>(
+ builder: (context, state, child) => GestureDetector(
Review comment:
May we consider wrapping this widget with a `MouseRegion` widget? Please see comment at https://github.com/apache/beam/pull/15709/files#diff-419ffd36df1c8677c1e9a82a1f59554182e415779c5e195053a4263f866e487eR35 for similar details.
--
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