From 6cb09201e07424cde070c1279af08cf4b1bf0c73 Mon Sep 17 00:00:00 2001 From: solid-maksymtielnyi Date: Thu, 2 Sep 2021 22:42:22 +0300 Subject: [PATCH 1/5] [WIP] Create design document for updating project to the latest Flutter version --- .../01_update_flutter_version_design.md | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md diff --git a/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md b/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md new file mode 100644 index 000000000..40e30cab4 --- /dev/null +++ b/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md @@ -0,0 +1,123 @@ +# Update flutter version + +> Summary of the proposed change. + +The latest versions of Flutter bring some changes to the existing API of the framework, so we will need to make some changes to the code to update our project to the latest version of Flutter successfully. + +# References + +> Link to supporting documentation, GitHub tickets, etc. + +* [`ElevatedButton` class](https://api.flutter.dev/flutter/material/ElevatedButton-class.html) +* [Use `BuildContext` synchronously linter rule](https://dart-lang.github.io/linter/lints/use_build_context_synchronously.html) +* [Use named constants linter rule](https://dart-lang.github.io/linter/lints/use_named_constants.html) + +# Motivation + +> What problem is this project solving? + +# Goals + +> Identify success metrics and measurable goals. + +* Metrics web project is updated to the latest Flutter version. +* Deprecated classes are not used in the project anymore. +* All functionalities present in the project work as expected. + +# Non-Goals + +> Identify what's not in scope. + +# Design + +> Explain and diagram the technical design. + +Consider the following changes to the existing code: +* Replace the `RaisedButton` class usages with `ElevatedButton`. To do this we should also make changes in button style declarations. For example: +```dart +Widget button() { + return RaisedButton( + color: loginOptionStyle.color, + disabledColor: loginOptionStyle.color, + hoverColor: loginOptionStyle.hoverColor, + elevation: loginOptionStyle.elevation, + hoverElevation: loginOptionStyle.elevation, + focusElevation: loginOptionStyle.elevation, + highlightElevation: loginOptionStyle.elevation, + shape: RoundedRectangleBorder( + borderRadius: BorderRadius.circular(4.0), + ), + ); +} +``` +should be replaced with the following: +```dart +Widget button() { + return ElevatedButton( + style: ButtonStyle( + backgroundColor: MaterialStateProperty.resolveWith((states) { + if (states.contains(MaterialState.disabled)) { + return loginOptionStyle.color; + } + if (states.contains(MaterialState.hovered)) { + return loginOptionStyle.hoverColor; + } + return loginOptionStyle.color; + }), + elevation: MaterialStateProperty.all(loginOptionStyle.elevation), + shape: MaterialStateProperty.all( + RoundedRectangleBorder( + borderRadius: BorderRadius.circular(4.0), + ), + ), + ), + ); +} +``` + +* Add checkout that the `State` is still present in the widget tree before using `BuildContext` inside async functions. For example: +```dart +Future asyncFunction() async { + if (mounted) { + Navigator.pop(context); + } +} +``` + +* Replace `const Duration()` usages with predefined constant `Duration.zero` to prevent duplicating its value. +* Replace `BorderRadius.all(Radius.zero)` usages with predefined constant `BorderRadius.zero` to prevent duplicating its value. + +# API + +> What will the proposed API look like? + +# Dependencies + +> What is the project blocked on? + +> What will be impacted by the project? + +# Testing + +> How will the project be tested? + +We should not add any new unit tests, widget tests, and integration tests because the expected behavior of the system will not change. However, we should apply changes mentioned in [Design](#design) section to unit tests to make them work properly with the latest Flutter version. +# Alternatives Considered + +> Summarize alternative designs (pros & cons). + +# Timeline + +> Document milestones and deadlines. + +DONE: + +- + +NEXT: + +- + +# Results + +> What was the outcome of the project? \ No newline at end of file From 9d69b093c0ee52b4ed66ac018186e074a86c6d7f Mon Sep 17 00:00:00 2001 From: solid-maksymtielnyi Date: Fri, 3 Sep 2021 22:18:23 +0300 Subject: [PATCH 2/5] Add analysis section to the 01_update_flutter_version_design.md document --- .../01_update_flutter_version_design.md | 180 ++++++++---------- 1 file changed, 82 insertions(+), 98 deletions(-) diff --git a/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md b/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md index 40e30cab4..49d3e6281 100644 --- a/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md +++ b/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md @@ -2,122 +2,106 @@ > Summary of the proposed change. -The latest versions of Flutter bring some changes to the existing API of the framework, so we will need to make some changes to the code to update our project to the latest version of Flutter successfully. +The latest versions of Flutter bring some changes to the existing API of the framework, so we should make some changes to the code to update our project to the latest version of Flutter successfully. # References > Link to supporting documentation, GitHub tickets, etc. +* [`RaisedButton` class](https://api.flutter.dev/flutter/material/RaisedButton-class.html) * [`ElevatedButton` class](https://api.flutter.dev/flutter/material/ElevatedButton-class.html) +* [Migrating to the New Material Buttons and their Themes](https://docs.google.com/document/d/1yohSuYrvyya5V1hB6j9pJskavCdVq9sVeTqSoEPsWH0/edit#heading=h.pub7jnop54q0) * [Use `BuildContext` synchronously linter rule](https://dart-lang.github.io/linter/lints/use_build_context_synchronously.html) -* [Use named constants linter rule](https://dart-lang.github.io/linter/lints/use_named_constants.html) - -# Motivation - -> What problem is this project solving? +* [Use named constants lint rule](https://dart-lang.github.io/linter/lints/use_named_constants.html) # Goals > Identify success metrics and measurable goals. -* Metrics web project is updated to the latest Flutter version. +* Metrics web application is updated to the latest Flutter version. * Deprecated classes are not used in the project anymore. -* All functionalities present in the project work as expected. - -# Non-Goals - -> Identify what's not in scope. - -# Design - -> Explain and diagram the technical design. - -Consider the following changes to the existing code: -* Replace the `RaisedButton` class usages with `ElevatedButton`. To do this we should also make changes in button style declarations. For example: -```dart -Widget button() { - return RaisedButton( - color: loginOptionStyle.color, - disabledColor: loginOptionStyle.color, - hoverColor: loginOptionStyle.hoverColor, - elevation: loginOptionStyle.elevation, - hoverElevation: loginOptionStyle.elevation, - focusElevation: loginOptionStyle.elevation, - highlightElevation: loginOptionStyle.elevation, - shape: RoundedRectangleBorder( - borderRadius: BorderRadius.circular(4.0), - ), - ); -} -``` -should be replaced with the following: -```dart -Widget button() { - return ElevatedButton( - style: ButtonStyle( - backgroundColor: MaterialStateProperty.resolveWith((states) { - if (states.contains(MaterialState.disabled)) { +* All functionalities present in the application work as expected. + +# Analysis + +> Describe a general analysis approach. + +If we update Flutter to the 2.2.3 stable version, we can see that the output of the dart analysis tool contains some warnings. To fix them, we should consider the following steps: +* The [`RaisedButton`](https://api.flutter.dev/flutter/material/RaisedButton-class.html) class is deprecated, so we should use the new [`ElevatedButton`](https://api.flutter.dev/flutter/material/ElevatedButton-class.html) class instead. To do this, we also should make changes in declarations of button styles according to the [Migrating to the New Material Buttons and their Themes](https://docs.google.com/document/d/1yohSuYrvyya5V1hB6j9pJskavCdVq9sVeTqSoEPsWH0/edit#heading=h.pub7jnop54q0) guide. For example: + ```dart + Widget button() { + return RaisedButton( + color: loginOptionStyle.color, + disabledColor: loginOptionStyle.color, + hoverColor: loginOptionStyle.hoverColor, + elevation: loginOptionStyle.elevation, + hoverElevation: loginOptionStyle.elevation, + focusElevation: loginOptionStyle.elevation, + highlightElevation: loginOptionStyle.elevation, + shape: RoundedRectangleBorder( + borderRadius: BorderRadius.circular(4.0), + ), + ); + } + ``` + should be replaced with the following: + ```dart + Widget button() { + return ElevatedButton( + style: ButtonStyle( + backgroundColor: MaterialStateProperty.resolveWith((states) { + if (states.contains(MaterialState.disabled)) { + return loginOptionStyle.color; + } + if (states.contains(MaterialState.hovered)) { + return loginOptionStyle.hoverColor; + } return loginOptionStyle.color; - } - if (states.contains(MaterialState.hovered)) { - return loginOptionStyle.hoverColor; - } - return loginOptionStyle.color; - }), - elevation: MaterialStateProperty.all(loginOptionStyle.elevation), - shape: MaterialStateProperty.all( - RoundedRectangleBorder( - borderRadius: BorderRadius.circular(4.0), + }), + elevation: MaterialStateProperty.all(loginOptionStyle.elevation), + shape: MaterialStateProperty.all( + RoundedRectangleBorder( + borderRadius: BorderRadius.circular(4.0), + ), ), ), - ), - ); -} -``` - -* Add checkout that the `State` is still present in the widget tree before using `BuildContext` inside async functions. For example: -```dart -Future asyncFunction() async { - if (mounted) { - Navigator.pop(context); + ); } -} -``` - -* Replace `const Duration()` usages with predefined constant `Duration.zero` to prevent duplicating its value. -* Replace `BorderRadius.all(Radius.zero)` usages with predefined constant `BorderRadius.zero` to prevent duplicating its value. - -# API - -> What will the proposed API look like? - -# Dependencies - -> What is the project blocked on? - -> What will be impacted by the project? - -# Testing - -> How will the project be tested? - -We should not add any new unit tests, widget tests, and integration tests because the expected behavior of the system will not change. However, we should apply changes mentioned in [Design](#design) section to unit tests to make them work properly with the latest Flutter version. -# Alternatives Considered - -> Summarize alternative designs (pros & cons). - -# Timeline - -> Document milestones and deadlines. - -DONE: - -- + ``` + We should make such changes in the following places in the code: + * `SignInOptionButton` class. + * `MetricsButton` class. + + +* Replace the `RaisedButton` usages with the `ElevatedButton` in the following files: + * `password_sign_in_option_test.dart`. + * `sign_in_option_button_test.dart`. + * `dropdown_body_test.dart`. + * `metrics_button_test.dart`. + * `injection_container_test.dart`. + * `delete_project_group_dialog_test.dart`. + * `project_group_dialog_test.dart`. + + +* Add checkout that the `State` is still present in the widget tree before using `BuildContext` inside async functions in accordance with [use_build_context_synchronously](https://dart-lang.github.io/linter/lints/use_build_context_synchronously.html) lint rule. For example: + ```dart + Future asyncFunction() async { + if (mounted) { + Navigator.pop(context); + } + } + ``` + This recommendation should be applied to the following files: + * `delete_project_group_dialog.dart`. + * `project_group_dialog.dart`. -NEXT: -- +* Replace `const Duration()` usages with predefined constant `Duration.zero` to prevent duplicating its value. This change is required in the following places in the code: + * `dropdown_body.dart` + * `performance_sparkline_view_model.dart` + * `build_result_bar_padding_strategy_test.dart` + + You can read more about this lint rule on the [use_named_constants](https://dart-lang.github.io/linter/lints/use_named_constants.html) page. -# Results -> What was the outcome of the project? \ No newline at end of file +* Replace `BorderRadius.all(Radius.zero)` usages with predefined constant `BorderRadius.zero` to prevent duplicating its value. Please, apply this change to the `material_container_test.dart` file. You can read more about this lint rule on the [use_named_constants](https://dart-lang.github.io/linter/lints/use_named_constants.html) page. From 271b5c041a11f6b9132012a5594366fc8d37a9cb Mon Sep 17 00:00:00 2001 From: solid-maksymtielnyi Date: Sat, 4 Sep 2021 13:42:58 +0300 Subject: [PATCH 3/5] Minor changes --- .../01_update_flutter_version_design.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md b/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md index 49d3e6281..aa5625a98 100644 --- a/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md +++ b/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md @@ -2,7 +2,7 @@ > Summary of the proposed change. -The latest versions of Flutter bring some changes to the existing API of the framework, so we should make some changes to the code to update our project to the latest version of Flutter successfully. +The latest versions of Flutter bring some changes to the existing API of the framework, so we should make some changes to the code to adapt our project to the latest version of Flutter. # References @@ -11,7 +11,7 @@ The latest versions of Flutter bring some changes to the existing API of the fra * [`RaisedButton` class](https://api.flutter.dev/flutter/material/RaisedButton-class.html) * [`ElevatedButton` class](https://api.flutter.dev/flutter/material/ElevatedButton-class.html) * [Migrating to the New Material Buttons and their Themes](https://docs.google.com/document/d/1yohSuYrvyya5V1hB6j9pJskavCdVq9sVeTqSoEPsWH0/edit#heading=h.pub7jnop54q0) -* [Use `BuildContext` synchronously linter rule](https://dart-lang.github.io/linter/lints/use_build_context_synchronously.html) +* [Use `BuildContext` synchronously lint rule](https://dart-lang.github.io/linter/lints/use_build_context_synchronously.html) * [Use named constants lint rule](https://dart-lang.github.io/linter/lints/use_named_constants.html) # Goals @@ -83,7 +83,7 @@ If we update Flutter to the 2.2.3 stable version, we can see that the output of * `project_group_dialog_test.dart`. -* Add checkout that the `State` is still present in the widget tree before using `BuildContext` inside async functions in accordance with [use_build_context_synchronously](https://dart-lang.github.io/linter/lints/use_build_context_synchronously.html) lint rule. For example: +* Add checkout that the `State` is still present in the widget tree before using `BuildContext` inside async functions in accordance with the [use_build_context_synchronously](https://dart-lang.github.io/linter/lints/use_build_context_synchronously.html) lint rule. For example: ```dart Future asyncFunction() async { if (mounted) { From cbda246c0260f398bf944d6074b4ebbc269d6b66 Mon Sep 17 00:00:00 2001 From: solid-maksymtielnyi Date: Tue, 7 Sep 2021 19:13:05 +0300 Subject: [PATCH 4/5] Add some code examples --- .../01_update_flutter_version_design.md | 170 ++++++++++++------ 1 file changed, 116 insertions(+), 54 deletions(-) diff --git a/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md b/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md index aa5625a98..f0ae7b15e 100644 --- a/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md +++ b/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md @@ -26,80 +26,142 @@ The latest versions of Flutter bring some changes to the existing API of the fra > Describe a general analysis approach. -If we update Flutter to the 2.2.3 stable version, we can see that the output of the dart analysis tool contains some warnings. To fix them, we should consider the following steps: -* The [`RaisedButton`](https://api.flutter.dev/flutter/material/RaisedButton-class.html) class is deprecated, so we should use the new [`ElevatedButton`](https://api.flutter.dev/flutter/material/ElevatedButton-class.html) class instead. To do this, we also should make changes in declarations of button styles according to the [Migrating to the New Material Buttons and their Themes](https://docs.google.com/document/d/1yohSuYrvyya5V1hB6j9pJskavCdVq9sVeTqSoEPsWH0/edit#heading=h.pub7jnop54q0) guide. For example: +If we update Flutter to the 2.2.3 stable version, we can see that the output of the dart analysis tool contains some warnings. The steps required to fix them are described below. +* The [`RaisedButton`](https://api.flutter.dev/flutter/material/RaisedButton-class.html) class is deprecated, so we should use the new [`ElevatedButton`](https://api.flutter.dev/flutter/material/ElevatedButton-class.html) class instead. To do this, we also should make changes in declarations of button styles according to the [Migrating to the New Material Buttons and their Themes](https://docs.google.com/document/d/1yohSuYrvyya5V1hB6j9pJskavCdVq9sVeTqSoEPsWH0/edit#heading=h.pub7jnop54q0) guide. To do this, please consider the following steps: + + 1. Add a `buttonStyle` getter to the `MetricsButtonStyle` class, which will return an object of `ButtonStyle` class with passed style parameters. For example: ```dart - Widget button() { - return RaisedButton( - color: loginOptionStyle.color, - disabledColor: loginOptionStyle.color, - hoverColor: loginOptionStyle.hoverColor, - elevation: loginOptionStyle.elevation, - hoverElevation: loginOptionStyle.elevation, - focusElevation: loginOptionStyle.elevation, - highlightElevation: loginOptionStyle.elevation, - shape: RoundedRectangleBorder( - borderRadius: BorderRadius.circular(4.0), - ), + /// A [ButtonStyle] with passed [color], [hoverColor], [labelStyle] and + /// [elevation] values. + ButtonStyle get buttonStyle => ButtonStyle( + backgroundColor: MaterialStateProperty.resolveWith((states) { + if (states.contains(MaterialState.disabled)) { + return color; + } + if (states.contains(MaterialState.hovered)) { + return hoverColor; + } + return color; + }), + elevation: MaterialStateProperty.all(elevation), + shape: MaterialStateProperty.all(shape), + textStyle: MaterialStateProperty.all(labelStyle), + ); + ``` + + 2. Replace usages of `RaisedButton` class with `ElevatedButton` in `SignInOptionButton` and `MetricsButton` classes. Let's review the changed `build()` method of `SignInOptionButton` class with `ElevatedButton` usage: + ```dart + @override + Widget build(BuildContext context) { + final metricsTheme = MetricsTheme.of(context); + + return Selector( + selector: (_, notifier) => notifier.isLoading, + builder: (_, isLoading, __) { + final loginOptionStyle = strategy.getWidgetAppearance( + metricsTheme, + isLoading, + ); + + return ElevatedButton( + style: loginOptionStyle.buttonStyle, + onPressed: isLoading ? null : () => _signIn(context), + child: Row( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + Padding( + padding: const EdgeInsets.only(right: 16.0), + child: SvgImage( + strategy.asset, + height: 20.0, + width: 20.0, + fit: BoxFit.contain, + ), + ), + Text( + strategy.label, + style: loginOptionStyle.labelStyle, + ), + ], + ), + ); + }, ); } ``` - should be replaced with the following: + Here is how the `build()` method of the `MetricsButton` class can look like: ```dart - Widget button() { - return ElevatedButton( - style: ButtonStyle( - backgroundColor: MaterialStateProperty.resolveWith((states) { - if (states.contains(MaterialState.disabled)) { - return loginOptionStyle.color; - } - if (states.contains(MaterialState.hovered)) { - return loginOptionStyle.hoverColor; - } - return loginOptionStyle.color; - }), - elevation: MaterialStateProperty.all(loginOptionStyle.elevation), - shape: MaterialStateProperty.all( - RoundedRectangleBorder( - borderRadius: BorderRadius.circular(4.0), - ), - ), + @override + Widget build(BuildContext context) { + final buttonTheme = MetricsTheme.of(context).metricsButtonTheme; + final attentionLevel = buttonTheme.attentionLevel; + final inactiveStyle = attentionLevel.inactive; + final style = selectStyle(attentionLevel); + + ElevatedButton( + style: style.buttonStyle, + onPressed: onPressed, + child: Text( + label, + style: onPressed == null ? inactiveStyle.labelStyle : style.labelStyle, ), ); } ``` - We should make such changes in the following places in the code: - * `SignInOptionButton` class. - * `MetricsButton` class. - -* Replace the `RaisedButton` usages with the `ElevatedButton` in the following files: - * `password_sign_in_option_test.dart`. - * `sign_in_option_button_test.dart`. - * `dropdown_body_test.dart`. - * `metrics_button_test.dart`. - * `injection_container_test.dart`. - * `delete_project_group_dialog_test.dart`. - * `project_group_dialog_test.dart`. + 3. Replace the `RaisedButton` usages with the `ElevatedButton` in the following files: + * `password_sign_in_option_test.dart`. + * `sign_in_option_button_test.dart`. + * `dropdown_body_test.dart`. + * `metrics_button_test.dart`. + * `injection_container_test.dart`. + * `delete_project_group_dialog_test.dart`. + * `project_group_dialog_test.dart`. -* Add checkout that the `State` is still present in the widget tree before using `BuildContext` inside async functions in accordance with the [use_build_context_synchronously](https://dart-lang.github.io/linter/lints/use_build_context_synchronously.html) lint rule. For example: +* Add check that the `State` is still present in the widget tree before using `BuildContext` inside async functions in accordance with the [use_build_context_synchronously](https://dart-lang.github.io/linter/lints/use_build_context_synchronously.html) lint rule. For example: ```dart - Future asyncFunction() async { - if (mounted) { + /// Starts deleting process of a project group. + Future _deleteProjectGroup( + DeleteProjectGroupDialogViewModel projectGroupDeleteDialogViewModel, + ) async { + final notifier = Provider.of(context, listen: false); + + _setLoading(true); + + await notifier.deleteProjectGroup(projectGroupDeleteDialogViewModel.id); + + final projectGroupSavingError = notifier.projectGroupSavingError; + + Toast toast; + + if (!mounted) { + return; + } + + if (projectGroupSavingError == null) { Navigator.pop(context); + final message = ProjectGroupsStrings.getDeletedProjectGroupMessage( + projectGroupDeleteDialogViewModel.name, + ); + toast = PositiveToast(message: message); + } else { + _setLoading(false); + toast = NegativeToast(message: projectGroupSavingError); } + + showToast(context, toast); } ``` - This recommendation should be applied to the following files: - * `delete_project_group_dialog.dart`. - * `project_group_dialog.dart`. + This recommendation should be applied to the following classes: + * `_DeleteProjectGroupDialogState` class, `_deleteProjectGroup()` method. + * `_ProjectGroupDialogState` class, `_actionCallback()` method. * Replace `const Duration()` usages with predefined constant `Duration.zero` to prevent duplicating its value. This change is required in the following places in the code: - * `dropdown_body.dart` - * `performance_sparkline_view_model.dart` - * `build_result_bar_padding_strategy_test.dart` + * `DropdownBody` class constructor. + * `PerformanceSparklineViewModel.dart` class constructor. + * `build_result_bar_padding_strategy_test.dart` tests. You can read more about this lint rule on the [use_named_constants](https://dart-lang.github.io/linter/lints/use_named_constants.html) page. From fd4cb09c5491d2e6b31e8f57c4fe5cfea9270c17 Mon Sep 17 00:00:00 2001 From: solid-maksymtielnyi Date: Wed, 8 Sep 2021 10:30:22 +0300 Subject: [PATCH 5/5] Minor fixes --- .../01_update_flutter_version_design.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md b/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md index f0ae7b15e..1b4aea41c 100644 --- a/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md +++ b/metrics/web/docs/features/update_flutter_version/01_update_flutter_version_design.md @@ -135,9 +135,7 @@ If we update Flutter to the 2.2.3 stable version, we can see that the output of Toast toast; - if (!mounted) { - return; - } + if (!mounted) return; if (projectGroupSavingError == null) { Navigator.pop(context);