From 79957c3217bccd97e9d519b9343a167d9dde2a32 Mon Sep 17 00:00:00 2001 From: lindatseng Date: Fri, 12 Apr 2019 10:42:09 -0700 Subject: [PATCH] Update panel logging to include all hide page cases The old logging only include see_more, done, and clicked_out when hiding the panel page. We were missing many cases that user might use back button, app switch button to close the page. Update the hide page keys to change the clicked_out to others to include all the other cases which hide the page. Test: Manual verification Test: atest PanelFragmentTest SettingsPanelActivityTest Fixes: 130169553 Change-Id: Icede9a8dcb84565cba183963c9fb554507631c98 --- .../android/settings/panel/PanelFragment.java | 32 ++++++++++++------- .../settings/panel/PanelLoggingContract.java | 5 +-- .../settings/panel/SettingsPanelActivity.java | 17 ---------- .../settings/panel/PanelFragmentTest.java | 10 ++++++ .../panel/SettingsPanelActivityTest.java | 2 +- 5 files changed, 34 insertions(+), 32 deletions(-) diff --git a/src/com/android/settings/panel/PanelFragment.java b/src/com/android/settings/panel/PanelFragment.java index f1391dc6d63..7f71925a5c2 100644 --- a/src/com/android/settings/panel/PanelFragment.java +++ b/src/com/android/settings/panel/PanelFragment.java @@ -19,6 +19,7 @@ package com.android.settings.panel; import android.app.settings.SettingsEnums; import android.content.Context; import android.os.Bundle; +import android.text.TextUtils; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; @@ -50,6 +51,7 @@ public class PanelFragment extends Fragment { private PanelContent mPanel; private MetricsFeatureProvider mMetricsProvider; + private String mPanelClosedKey; @VisibleForTesting PanelSlicesAdapter mAdapter; @@ -111,15 +113,26 @@ public class PanelFragment extends Fragment { return view; } + @Override + public void onDestroyView() { + super.onDestroyView(); + + if (TextUtils.isEmpty(mPanelClosedKey)) { + mPanelClosedKey = PanelClosedKeys.KEY_OTHERS; + } + + mMetricsProvider.action( + 0 /* attribution */, + SettingsEnums.PAGE_HIDE, + mPanel.getMetricsCategory(), + mPanelClosedKey, + 0 /* value */); + } + @VisibleForTesting View.OnClickListener getSeeMoreListener() { return (v) -> { - mMetricsProvider.action( - 0 /* attribution */, - SettingsEnums.PAGE_HIDE , - mPanel.getMetricsCategory(), - PanelClosedKeys.KEY_SEE_MORE, - 0 /* value */); + mPanelClosedKey = PanelClosedKeys.KEY_SEE_MORE; final FragmentActivity activity = getActivity(); activity.startActivityForResult(mPanel.getSeeMoreIntent(), 0); activity.finish(); @@ -129,12 +142,7 @@ public class PanelFragment extends Fragment { @VisibleForTesting View.OnClickListener getCloseListener() { return (v) -> { - mMetricsProvider.action( - 0 /* attribution */, - SettingsEnums.PAGE_HIDE, - mPanel.getMetricsCategory(), - PanelClosedKeys.KEY_DONE, - 0 /* value */); + mPanelClosedKey = PanelClosedKeys.KEY_DONE; getActivity().finish(); }; } diff --git a/src/com/android/settings/panel/PanelLoggingContract.java b/src/com/android/settings/panel/PanelLoggingContract.java index e14918648fd..e6e3012abef 100644 --- a/src/com/android/settings/panel/PanelLoggingContract.java +++ b/src/com/android/settings/panel/PanelLoggingContract.java @@ -39,8 +39,9 @@ public class PanelLoggingContract { String KEY_DONE = "done"; /** - * The user clicked outside the dialog, closing the Panel. + * The user closed the panel by other ways, for example: clicked outside of dialog, tapping + * on back button, etc. */ - String KEY_CLICKED_OUT = "clicked_out"; + String KEY_OTHERS = "others"; } } diff --git a/src/com/android/settings/panel/SettingsPanelActivity.java b/src/com/android/settings/panel/SettingsPanelActivity.java index 8aee38238d8..eabd7151fc8 100644 --- a/src/com/android/settings/panel/SettingsPanelActivity.java +++ b/src/com/android/settings/panel/SettingsPanelActivity.java @@ -97,21 +97,4 @@ public class SettingsPanelActivity extends FragmentActivity { fragmentManager.beginTransaction().add(R.id.main_content, panelFragment).commit(); } } - - @Override - public boolean onTouchEvent(MotionEvent event) { - if (event.getAction() == MotionEvent.ACTION_OUTSIDE) { - final PanelContent panelContent = FeatureFactory.getFactory(this) - .getPanelFeatureProvider() - .getPanel(this, getIntent().getAction(), null /* Media Package Name */); - FeatureFactory.getFactory(this) - .getMetricsFeatureProvider() - .action(0 /* attribution */, - SettingsEnums.PAGE_HIDE, - panelContent.getMetricsCategory(), - PanelClosedKeys.KEY_CLICKED_OUT, - 0 /* value */); - } - return super.onTouchEvent(event); - } } diff --git a/tests/robotests/src/com/android/settings/panel/PanelFragmentTest.java b/tests/robotests/src/com/android/settings/panel/PanelFragmentTest.java index be8d8bc72b4..d606ac7ad65 100644 --- a/tests/robotests/src/com/android/settings/panel/PanelFragmentTest.java +++ b/tests/robotests/src/com/android/settings/panel/PanelFragmentTest.java @@ -99,6 +99,16 @@ public class PanelFragmentTest { 0); } + @Test + public void onDestroy_logCloseEvent() { + mPanelFragment.onDestroy(); + verify(mFakeFeatureFactory.metricsFeatureProvider).action( + 0, + SettingsEnums.PAGE_VISIBLE, + mFakePanelContent.getMetricsCategory(), + any(String.class), + 0); } + @Test public void panelSeeMoreClick_logsCloseEvent() { final View.OnClickListener listener = mPanelFragment.getSeeMoreListener(); diff --git a/tests/robotests/src/com/android/settings/panel/SettingsPanelActivityTest.java b/tests/robotests/src/com/android/settings/panel/SettingsPanelActivityTest.java index 1d5c3c28cee..fa15aa02b1d 100644 --- a/tests/robotests/src/com/android/settings/panel/SettingsPanelActivityTest.java +++ b/tests/robotests/src/com/android/settings/panel/SettingsPanelActivityTest.java @@ -99,7 +99,7 @@ public class SettingsPanelActivityTest { 0, SettingsEnums.PAGE_HIDE, SettingsEnums.TESTING, - PanelLoggingContract.PanelClosedKeys.KEY_CLICKED_OUT, + PanelLoggingContract.PanelClosedKeys.KEY_OTHERS, 0 ); }