alexeyinkin commented on code in PR #24957:
URL: https://github.com/apache/beam/pull/24957#discussion_r1065782592
##########
playground/frontend/lib/pages/standalone_playground/widgets/feedback/feedback_dropdown_content.dart:
##########
@@ -172,9 +174,16 @@ class FeedbackDropdownContent extends StatelessWidget {
key: sendButtonKey,
onPressed: () {
if (textController.text.isNotEmpty) {
- AnalyticsService.get(context).trackClickSendFeedback(
- textController.text,
- );
+ final text = textController.text;
+ if (isPositive) {
+
AnalyticsService.get().trackClickSendPositiveFeedback(
+ text,
+ );
+ } else {
Review Comment:
Second thoughts, this is too much of investment into boolean. I suggest
making an `enum FeedbackRating` with `positive` and `negative`.
##########
playground/frontend/lib/modules/analytics/service.dart:
##########
@@ -16,16 +16,12 @@
* limitations under the License.
*/
-import 'package:flutter/widgets.dart';
-import 'package:playground/modules/analytics/analytics_event.dart';
+import 'package:get_it/get_it.dart';
import 'package:playground_components/playground_components.dart';
-import 'package:provider/provider.dart';
-abstract class AnalyticsService {
- AnalyticsEvent? get lastSentEvent;
-
- static AnalyticsService get(BuildContext context) {
- return Provider.of<AnalyticsService>(context, listen: false);
+abstract class AnalyticsService extends GeneralAnalyticsService {
Review Comment:
How about this?
```
AnalyticsService -> PlaygroundAnalyticsService ->
PlaygroundGoogleAnalyticsService
-> TobAnalyticsService -> TobGoogleAnalyticsService
```
##########
playground/frontend/lib/locator.dart:
##########
@@ -57,3 +59,7 @@ void _initializeState() {
),
);
}
+
+void _initializeAnalyticsService() {
Review Comment:
```suggestion
void _initializeServices() {
```
##########
playground/frontend/lib/locator.dart:
##########
@@ -57,3 +59,7 @@ void _initializeState() {
),
);
}
+
+void _initializeAnalyticsService() {
+ GetIt.instance.registerSingleton(GoogleAnalyticsService());
Review Comment:
You can register a single object under two handles:
```suggestion
GetIt.instance.registerSingleton<AnalyticsService>(service);
GetIt.instance.registerSingleton<PlaygroundAnalyticsService>(service);
```
This way `playground_components` will request `<AnalyticsService>` and be
able to submit generic events, while Playground and ToB will fetch their own
subclasses to submit their specific events.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
