You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "nausharipov (via GitHub)" <gi...@apache.org> on 2023/02/06 16:43:58 UTC
[GitHub] [beam] nausharipov opened a new pull request, #25347: [Tour of Beam] [Frontend] Delete account
nausharipov opened a new pull request, #25347:
URL: https://github.com/apache/beam/pull/25347
Resolves #25255
------------------------
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 #25347: [Tour of Beam] [Frontend] Delete account
Posted by "alexeyinkin (via GitHub)" <gi...@apache.org>.
alexeyinkin commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1098487731
##########
learning/tour-of-beam/frontend/lib/components/profile/user_menu.dart:
##########
@@ -125,9 +125,29 @@ class _Buttons extends StatelessWidget {
),
const BeamDivider(),
_IconLabel(
- onTap: () {},
+ onTap: () {
+ closeOverlayCallback();
+ showDialog(
+ context: context,
+ builder: (context) => Dialog(
+ backgroundColor: Colors.transparent,
+ child: BeamAlertDialog(
+ body: 'dialogs.deleteAccountWarning'.tr(),
+ continueLabel: 'ui.deleteMyAccount'.tr(),
+ title: 'ui.deleteTobAccount'.tr(),
+ onContinue: () {
+ authNotifier.deleteAccount().then(
+ (_) {
+ Navigator.pop(context);
+ },
+ );
+ },
Review Comment:
1. Oh, by the way this is just the same problem, but the linter cannot spot it. One solution is to let the dialog close itself by passing a callback to `onContinue`, but this is ugly. Try to think of better ideas. Also this is exactly why in app_state we have states pushing themselves out without a context.
2. Yes.
--
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 #25347: [Tour of Beam] [Frontend] Delete account
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1101470914
##########
playground/frontend/playground_components/lib/src/widgets/overlay/opener.dart:
##########
@@ -19,17 +19,19 @@
import 'package:flutter/material.dart';
import '../../controllers/public_notifier.dart';
-import 'dismissible.dart';
+import 'widget.dart';
void openOverlay({
required BuildContext context,
required PublicNotifier closeNotifier,
required Positioned positioned,
+ bool isDismissible = true,
Review Comment:
The current users of overlay (LoginButton and Avatar) expect it to be dismissible. Also, it is consistent with showDialog's `bool barrierDismissible = true`.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102443853
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
+ final VoidCallback onContinue;
+ final String title;
Review Comment:
Wrapped the dialog in `SingleChildScrollView` to avoid overflow.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102421275
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
Review Comment:
Added a 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] nausharipov commented on a diff in pull request #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102464547
##########
playground/frontend/playground_components/lib/src/widgets/overlay/overlays.dart:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 '../../../playground_components.dart';
+
+class BeamOverlays {
+ static Future<void> showProgressOverlay(
Review Comment:
Added a 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] nausharipov commented on a diff in pull request #25347: [Tour of Beam] [Frontend] Delete account
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1098351047
##########
playground/frontend/playground_components/lib/src/widgets/popups/alert_dialog.dart:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.
+ */
+
Review Comment:
Deleted /popups.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102421275
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
Review Comment:
Added a comment and renamed the widget.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "alexeyinkin (via GitHub)" <gi...@apache.org>.
alexeyinkin commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1105722165
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/progress.dart:
##########
@@ -16,29 +16,43 @@
* limitations under the License.
*/
+import 'dart:async';
+
import 'package:flutter/material.dart';
-import '../../../playground_components.dart';
+class ProgressDialog extends StatelessWidget {
Review Comment:
Move the comment from the overlay.
--
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 #25347: [Tour of Beam] [Frontend] Delete account
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1098313760
##########
learning/tour-of-beam/frontend/lib/components/profile/user_menu.dart:
##########
@@ -125,9 +125,29 @@ class _Buttons extends StatelessWidget {
),
const BeamDivider(),
_IconLabel(
- onTap: () {},
+ onTap: () {
+ closeOverlayCallback();
+ showDialog(
+ context: context,
+ builder: (context) => Dialog(
+ backgroundColor: Colors.transparent,
+ child: BeamAlertDialog(
+ body: 'dialogs.deleteAccountWarning'.tr(),
+ continueLabel: 'ui.deleteMyAccount'.tr(),
+ title: 'ui.deleteTobAccount'.tr(),
+ onContinue: () {
+ authNotifier.deleteAccount().then(
+ (_) {
+ Navigator.pop(context);
+ },
+ );
+ },
Review Comment:
1. Linter is against using context in async functions.
2. Do you mean showing an overlaying loading animation?
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102420884
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
Review Comment:
Renamed `text` to `subtitle`. It is optional, because some dialogs might not need body text.
--
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 pull request #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on PR #25347:
URL: https://github.com/apache/beam/pull/25347#issuecomment-1425999126
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] nausharipov commented on a diff in pull request #25347: [Tour of Beam] [Frontend] Delete account
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1101621044
##########
playground/frontend/playground_components/lib/src/widgets/overlay/opener.dart:
##########
@@ -19,17 +19,19 @@
import 'package:flutter/material.dart';
import '../../controllers/public_notifier.dart';
-import 'dismissible.dart';
+import 'widget.dart';
void openOverlay({
required BuildContext context,
required PublicNotifier closeNotifier,
required Positioned positioned,
+ bool isDismissible = true,
Review Comment:
I renamed the function to `showOverlay` and the parameter to `barrierDismissible` for consistency, still assuming that `true` is the preferable setting.
--
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 pull request #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on PR #25347:
URL: https://github.com/apache/beam/pull/25347#issuecomment-1424376549
Switching between hints and solutions is done in #25298.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102443853
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
+ final VoidCallback onContinue;
+ final String title;
Review Comment:
Wrapped the dialog content in `SingleChildScrollView` to avoid overflow.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102441416
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
Review Comment:
Renamed `continueLabel`.
--
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 #25347: [Tour of Beam] [Frontend] Delete account
Posted by "alexeyinkin (via GitHub)" <gi...@apache.org>.
alexeyinkin commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1098498691
##########
learning/tour-of-beam/frontend/lib/components/profile/user_menu.dart:
##########
@@ -125,9 +125,29 @@ class _Buttons extends StatelessWidget {
),
const BeamDivider(),
_IconLabel(
- onTap: () {},
+ onTap: () {
+ closeOverlayCallback();
+ showDialog(
+ context: context,
+ builder: (context) => Dialog(
+ backgroundColor: Colors.transparent,
+ child: BeamAlertDialog(
+ body: 'dialogs.deleteAccountWarning'.tr(),
+ continueLabel: 'ui.deleteMyAccount'.tr(),
+ title: 'ui.deleteTobAccount'.tr(),
+ onContinue: () {
+ authNotifier.deleteAccount().then(
+ (_) {
+ Navigator.pop(context);
+ },
+ );
+ },
Review Comment:
The simplest solution is to first pop the dialog and then to delete the account.
--
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 closed pull request #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov closed pull request #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
URL: https://github.com/apache/beam/pull/25347
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102441416
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
Review Comment:
Renamed `continueLabel` to `continueButtonText`.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102461927
##########
playground/frontend/playground_components/lib/src/widgets/overlay/overlays.dart:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 '../../../playground_components.dart';
+
+class BeamOverlays {
Review Comment:
Added a 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] nausharipov commented on a diff in pull request #25347: [Tour of Beam] [Frontend] Delete account
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1098351047
##########
playground/frontend/playground_components/lib/src/widgets/popups/alert_dialog.dart:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.
+ */
+
Review Comment:
From /dialogs.
--
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 #25347: [Tour of Beam] [Frontend] Delete account
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1098348963
##########
learning/tour-of-beam/frontend/lib/pages/tour/widgets/solution_button.dart:
##########
@@ -42,7 +43,23 @@ class SolutionButton extends StatelessWidget {
: null,
),
),
- onPressed: tourNotifier.toggleShowingSolution,
+ onPressed: () {
+ // TODO(nausharipov): resolve the conflict with save user code
+ showDialog(
+ context: context,
+ builder: (context) => Dialog(
+ backgroundColor: Colors.transparent,
+ child: BeamAlertDialog(
Review Comment:
Moved `Dialog` into BeamAlertDialog, because future users of the widget might not know about the
```
Future<void> show(BuildContext context) async {
await showDialog(
context: context,
builder: build,
);
}
```
function and use showDialog.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102420884
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
Review Comment:
Renamed `text` to `bodyText`. It is optional, because some dialogs might not need body text.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102457687
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
+ final VoidCallback onContinue;
Review Comment:
Added the route pop and a 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] nausharipov commented on a diff in pull request #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102441416
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
Review Comment:
Renamed the parameter.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25347:
URL: https://github.com/apache/beam/pull/25347#issuecomment-1424437204
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] damondouglas commented on a diff in pull request #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1101743995
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
Review Comment:
Could you comment what this Widget does and why it exists? It's somewhat obvious that it is a general purpose dialog Widget but it would be helpful to others needing to change it in the future at least one sentence describing it.
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
Review Comment:
What does continueLabel do? Can you add a comment?
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
+ final VoidCallback onContinue;
+ final String title;
Review Comment:
Is there a character limit?
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
+ final VoidCallback onContinue;
Review Comment:
Comment perhaps that the route is popped, etc.
##########
playground/frontend/playground_components/lib/src/widgets/overlay/overlays.dart:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 '../../../playground_components.dart';
+
+class BeamOverlays {
Review Comment:
The fact that it is plural stages future types of overlay widgets? Could you comment on the purpose of this class?
##########
playground/frontend/playground_components/lib/src/widgets/overlay/overlays.dart:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 '../../../playground_components.dart';
+
+class BeamOverlays {
+ static Future<void> showProgressOverlay(
Review Comment:
I'm assuming the purpose of this is to simply overlay a CircularProgressIndicator? Could you add a comment?
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
Review Comment:
Comment that (I'm assuming) `test` lands in the body of the dialog widget? Why is it optional?
--
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 #25347: [Tour of Beam] [Frontend] Delete account
Posted by "alexeyinkin (via GitHub)" <gi...@apache.org>.
alexeyinkin commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1098223869
##########
learning/tour-of-beam/frontend/lib/components/profile/user_menu.dart:
##########
@@ -125,9 +125,29 @@ class _Buttons extends StatelessWidget {
),
const BeamDivider(),
_IconLabel(
- onTap: () {},
+ onTap: () {
+ closeOverlayCallback();
+ showDialog(
+ context: context,
+ builder: (context) => Dialog(
+ backgroundColor: Colors.transparent,
+ child: BeamAlertDialog(
+ body: 'dialogs.deleteAccountWarning'.tr(),
+ continueLabel: 'ui.deleteMyAccount'.tr(),
+ title: 'ui.deleteTobAccount'.tr(),
+ onContinue: () {
+ authNotifier.deleteAccount().then(
+ (_) {
+ Navigator.pop(context);
+ },
+ );
+ },
Review Comment:
1. async / await.
2. Show progress?
##########
learning/tour-of-beam/frontend/lib/pages/tour/widgets/solution_button.dart:
##########
@@ -42,7 +43,23 @@ class SolutionButton extends StatelessWidget {
: null,
),
),
- onPressed: tourNotifier.toggleShowingSolution,
+ onPressed: () {
+ // TODO(nausharipov): resolve the conflict with save user code
+ showDialog(
+ context: context,
+ builder: (context) => Dialog(
+ backgroundColor: Colors.transparent,
+ child: BeamAlertDialog(
Review Comment:
Duplicate code.
`BeamAlertDialog.show(context, ...);` ?
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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 '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? body;
+ final String continueLabel;
+ final VoidCallback onContinue;
+ final String title;
+
+ const BeamAlertDialog({
+ this.body,
+ required this.continueLabel,
+ required this.onContinue,
+ required this.title,
Review Comment:
Put required parameters first.
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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 '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? body;
+ final String continueLabel;
+ final VoidCallback onContinue;
+ final String title;
+
+ const BeamAlertDialog({
+ this.body,
+ required this.continueLabel,
+ required this.onContinue,
+ required this.title,
+ });
+
+ @override
+ Widget build(BuildContext context) {
+ return OverlayBody(
+ child: Container(
+ width: BeamSizes.popupWidth,
+ padding: const EdgeInsets.all(BeamSizes.size16),
+ child: Column(
+ crossAxisAlignment: CrossAxisAlignment.start,
+ mainAxisSize: MainAxisSize.min,
+ children: [
+ Text(
+ title,
+ style: Theme.of(context).textTheme.headlineMedium,
+ ),
+ if (body != null)
+ Padding(
+ padding: const EdgeInsets.only(top: BeamSizes.size8),
+ child: Text(body!),
+ ),
+ const SizedBox(height: BeamSizes.size8),
+ Row(
+ mainAxisAlignment: MainAxisAlignment.end,
+ children: [
+ TextButton(
+ onPressed: () {
+ Navigator.pop(context);
+ },
+ // TODO(nausharipov): review: translate in PGC?
Review Comment:
Yes, we have easy_localization there already and slowly migrating strings there.
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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 '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? body;
Review Comment:
```suggestion
final String? text;
```
?
##########
playground/frontend/playground_components/lib/src/widgets/popups/alert_dialog.dart:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.
+ */
+
Review Comment:
Which of these two files is used?
--
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 pull request #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on PR #25347:
URL: https://github.com/apache/beam/pull/25347#issuecomment-1430052437
"Playground Frontend Test / Playground Frontend Test (pull_request)" task is failing, because the fluttertoast package requires Flutter v3.7. Alexey added support for v3.7.3 to Playground in other PRs. I will be able to add the support after I make the v3.7.3 work on my computer.
--
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 #25347: [Tour of Beam] [Frontend] Delete account
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1098313760
##########
learning/tour-of-beam/frontend/lib/components/profile/user_menu.dart:
##########
@@ -125,9 +125,29 @@ class _Buttons extends StatelessWidget {
),
const BeamDivider(),
_IconLabel(
- onTap: () {},
+ onTap: () {
+ closeOverlayCallback();
+ showDialog(
+ context: context,
+ builder: (context) => Dialog(
+ backgroundColor: Colors.transparent,
+ child: BeamAlertDialog(
+ body: 'dialogs.deleteAccountWarning'.tr(),
+ continueLabel: 'ui.deleteMyAccount'.tr(),
+ title: 'ui.deleteTobAccount'.tr(),
+ onContinue: () {
+ authNotifier.deleteAccount().then(
+ (_) {
+ Navigator.pop(context);
+ },
+ );
+ },
Review Comment:
1. Linter is against using context in async functions.
2. Is a status enum in AuthNotifier the simplest solution for such behavior?
--
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 pull request #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on PR #25347:
URL: https://github.com/apache/beam/pull/25347#issuecomment-1424368197
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] nausharipov commented on a diff in pull request #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102457687
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
+ final VoidCallback onContinue;
Review Comment:
Renamed the parameter.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102421275
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
Review Comment:
Renamed the widget.
--
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 #25347: [Tour of Beam] [Frontend] Delete account
Posted by "alexeyinkin (via GitHub)" <gi...@apache.org>.
alexeyinkin commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1101271893
##########
playground/frontend/playground_components/lib/src/widgets/overlay/opener.dart:
##########
@@ -19,17 +19,19 @@
import 'package:flutter/material.dart';
import '../../controllers/public_notifier.dart';
-import 'dismissible.dart';
+import 'widget.dart';
void openOverlay({
required BuildContext context,
required PublicNotifier closeNotifier,
required Positioned positioned,
+ bool isDismissible = true,
Review Comment:
Why not `false`?
##########
playground/frontend/playground_components/lib/src/widgets/overlay/widget.dart:
##########
@@ -18,24 +18,27 @@
import 'package:flutter/material.dart';
-class DismissibleOverlay extends StatelessWidget {
+class BeamOverlay extends StatelessWidget {
final VoidCallback close;
final Positioned child;
+ final bool isDismissible;
- const DismissibleOverlay({
+ const BeamOverlay({
required this.close,
required this.child,
+ required this.isDismissible,
Review Comment:
Sort.
##########
playground/frontend/playground_components/lib/src/widgets/overlay/widget.dart:
##########
@@ -18,24 +18,27 @@
import 'package:flutter/material.dart';
-class DismissibleOverlay extends StatelessWidget {
+class BeamOverlay extends StatelessWidget {
final VoidCallback close;
final Positioned child;
+ final bool isDismissible;
Review Comment:
Sort.
##########
playground/frontend/playground_components/lib/src/widgets/overlay/overlays.dart:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 '../../../playground_components.dart';
+
+class BeamOverlays {
+ // TODO(nausharipov) review: add label?
+ // TODO(nausharipov) review: add grey-ish background?
Review Comment:
1. No.
2. Yes.
--
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 #25347: [Tour of Beam] [Frontend] Delete account
Posted by "alexeyinkin (via GitHub)" <gi...@apache.org>.
alexeyinkin commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1101603728
##########
playground/frontend/playground_components/lib/src/widgets/overlay/opener.dart:
##########
@@ -19,17 +19,19 @@
import 'package:flutter/material.dart';
import '../../controllers/public_notifier.dart';
-import 'dismissible.dart';
+import 'widget.dart';
void openOverlay({
required BuildContext context,
required PublicNotifier closeNotifier,
required Positioned positioned,
+ bool isDismissible = true,
Review Comment:
Rename it to `barrierDismissible` then?
I would prefer this to be required though because there is actually no good reason to prefer one over another.
--
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 #25347: [Tour of Beam] [Frontend] Delete account, GitHub auth, solution warning
Posted by "nausharipov (via GitHub)" <gi...@apache.org>.
nausharipov commented on code in PR #25347:
URL: https://github.com/apache/beam/pull/25347#discussion_r1102441416
##########
playground/frontend/playground_components/lib/src/widgets/dialogs/alert_dialog.dart:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+
+import '../../../playground_components.dart';
+
+class BeamAlertDialog extends StatelessWidget {
+ final String? text;
+ final String continueLabel;
Review Comment:
Renamed `continueLabel` to `approvalButtonText`.
--
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