Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-2100, GH-2097: Onboarding test 2 #603

Merged
merged 7 commits into from Sep 22, 2020
Merged

GH-2100, GH-2097: Onboarding test 2 #603

merged 7 commits into from Sep 22, 2020

Conversation

@leuryr
Copy link
Contributor

@leuryr leuryr commented Sep 14, 2020

Includes the addition of a new A/B test, which displays a different Hub layout on install for users.

  • The A/B/C test for the Hub promo variant has been removed
  • The ping param being used for the Hub promo variant has been repurposed with new values for Hub Layout test pings
  • The A/B test timing bug fix from PR #601 has been implemented here as well

  • Have you followed the guidelines in CONTRIBUTING.md?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what your changes do?
  • Does your submission pass tests?
  • Did you lint your code prior to submission?
@leuryr leuryr requested review from benstrumeyer, christophertino, wlycdgr and ghostery/extension as code owners Sep 14, 2020
@wlycdgr
Copy link
Member

@wlycdgr wlycdgr commented Sep 15, 2020

One bug I noticed: in the alternate Hub scenario, clicking on the Ghostery icon at the top of the sidebar makes the "this is the tab you're in" triangle indicator disappear

EDIT: Moot / resolved because icon now links to website

Copy link
Member

@wlycdgr wlycdgr left a comment

A few small things + we MAY need to keep the Metrics parts of the code for the old A/B test. Even if not, we should use a new Metrics query param name for this new test instead of hp, so we'll need a quick Metrics server ticket to add that param.

import SideNavigationView from './SideNavigationView';
import globals from '../../../../src/classes/Globals';

const { IS_CLIQZ, BROWSER_INFO } = globals;
const IS_ANDROID = (BROWSER_INFO.os === 'android');

// Flag to display alternate hub view (used in A/B testing)

This comment has been minimized.

@wlycdgr

wlycdgr Sep 15, 2020
Member

Same deal as above - could we add a reference to the specific ticket(s)?

src/background.js Outdated Show resolved Hide resolved
@wlycdgr
Copy link
Member

@wlycdgr wlycdgr commented Sep 15, 2020

NOTE: This PR should not be merged into develop until after 8.5.3 has been released

EDIT: Change of plan. This should go into 8.5.3.

leuryr added 2 commits Sep 16, 2020
@wlycdgr wlycdgr changed the title GH-2100, GH-2097: Onboarding test 2 GH-2100, GH-2097: Onboarding test 2 (MERGE AFTER 8.5.3 RELEASE) Sep 18, 2020
@christophertino christophertino added this to the 8.5.4 milestone Sep 21, 2020
@wlycdgr wlycdgr changed the title GH-2100, GH-2097: Onboarding test 2 (MERGE AFTER 8.5.3 RELEASE) GH-2100, GH-2097: Onboarding test 2 Sep 22, 2020
@christophertino christophertino modified the milestones: 8.5.4, 8.5.3 Sep 22, 2020
@christophertino christophertino merged commit f1c8941 into develop Sep 22, 2020
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@christophertino christophertino deleted the onboarding-test-2 branch Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants