From 287601a03d118290756625fe827b4bc40a9c3f48 Mon Sep 17 00:00:00 2001
From: Matthew Riley MacPherson <matt@lonelyvegan.com>
Date: Thu, 15 Jun 2017 17:47:17 +0100
Subject: [PATCH 1/2] fix: Log in button works on first click

Fixes #1904
---
 .../components/AuthenticateButton/index.js    | 12 ++++++++-
 src/core/css/inc/mixins.scss                  | 13 ++--------
 tests/unit/amo/helpers.js                     | 19 +++++++++++---
 .../core/components/TestAuthenticateButton.js | 25 ++++++++++++-------
 4 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/src/core/components/AuthenticateButton/index.js b/src/core/components/AuthenticateButton/index.js
index b41d37a5670..e1f261a9e3f 100644
--- a/src/core/components/AuthenticateButton/index.js
+++ b/src/core/components/AuthenticateButton/index.js
@@ -60,8 +60,18 @@ export class AuthenticateButtonBase extends React.Component {
     const buttonText = isAuthenticated ?
       logOutText || i18n.gettext('Log out') :
       logInText || i18n.gettext('Log in/Sign up');
+
+    // The `href` is required because a <button> element with a :hover effect
+    // and/or focus effect (that is not part of a form) that changes its
+    // appearance will transition to the hover state onClick when using a
+    // mobile browser. This is the cause of
+    // https://github.com/mozilla/addons-frontend/issues/1904
+    //
+    // We tried to use https://css-tricks.com/annoying-mobile-double-tap-link-issue/#article-header-id-2
+    // and media queries to fix this but the `href` solution worked so I'm
+    // going with this.
     return (
-      <Button onClick={this.onClick} {...otherProps}>
+      <Button onClick={this.onClick} href="#" {...otherProps}>
         {noIcon ? null : <Icon name="user-dark" />}
         {buttonText}
       </Button>
diff --git a/src/core/css/inc/mixins.scss b/src/core/css/inc/mixins.scss
index 2f39f2f2b85..64f47335dce 100644
--- a/src/core/css/inc/mixins.scss
+++ b/src/core/css/inc/mixins.scss
@@ -219,17 +219,8 @@ $default-arrow-margin: 3px;
     font-weight: 400;
   }
 
-  // This is required because a <button> element with a :hover effect
-  // that changes its appearance will transition to the hover state onClick
-  // when using a mobile browser.
-  // This is the cause of
-  // https://github.com/mozilla/addons-frontend/issues/1904
-  //
-  // For more info, see: https://css-tricks.com/annoying-mobile-double-tap-link-issue/#article-header-id-2
-  @media (pointer: fine) {
-    &:hover {
-      background: $background-hover;
-    }
+  &:hover {
+    background: $background-hover;
   }
 
   &:active,
diff --git a/tests/unit/amo/helpers.js b/tests/unit/amo/helpers.js
index 2ea73654f50..4443ec9ce19 100644
--- a/tests/unit/amo/helpers.js
+++ b/tests/unit/amo/helpers.js
@@ -66,17 +66,30 @@ export const signedInApiState = Object.freeze({
   clientApp: 'firefox',
 });
 
-export function dispatchSignInActions({
+export function dispatchAnonymousActions({
   store = createStore().store,
   clientApp = 'android',
-  authToken = userAuthToken(),
   lang = 'en-US',
   userAgent = sampleUserAgent,
 } = {}) {
-  store.dispatch(setAuthToken(authToken));
   store.dispatch(setClientApp(clientApp));
   store.dispatch(setLang(lang));
   store.dispatch(setUserAgent(userAgent));
+
+  return {
+    store,
+    state: store.getState(),
+  };
+}
+
+export function dispatchSignInActions({
+  authToken = userAuthToken(),
+  ...otherArgs
+} = {}) {
+  const { store } = dispatchAnonymousActions(otherArgs);
+
+  store.dispatch(setAuthToken(authToken));
+
   return {
     store,
     state: store.getState(),
diff --git a/tests/unit/core/components/TestAuthenticateButton.js b/tests/unit/core/components/TestAuthenticateButton.js
index 490c0b430bd..8a34cb531d7 100644
--- a/tests/unit/core/components/TestAuthenticateButton.js
+++ b/tests/unit/core/components/TestAuthenticateButton.js
@@ -4,7 +4,7 @@ import {
   Simulate, findRenderedComponentWithType, renderIntoDocument,
 } from 'react-addons-test-utils';
 import { findDOMNode } from 'react-dom';
-import { combineReducers, createStore as _createStore } from 'redux';
+import { Provider } from 'react-redux';
 
 import { setAuthToken } from 'core/actions';
 import * as api from 'core/api';
@@ -13,17 +13,24 @@ import {
   mapDispatchToProps,
   mapStateToProps,
 } from 'core/components/AuthenticateButton';
-import apiReducer from 'core/reducers/api';
+import {
+  dispatchAnonymousActions,
+  dispatchSignInActions,
+} from 'tests/unit/amo/helpers';
 import { getFakeI18nInst, userAuthToken } from 'tests/unit/helpers';
 import Icon from 'ui/components/Icon';
 
-function createStore() {
-  return { store: _createStore(combineReducers({ api: apiReducer })) };
-}
 
 describe('<AuthenticateButton />', () => {
-  const renderTree = (props) => renderIntoDocument(
-    <AuthenticateButtonBase i18n={getFakeI18nInst()} {...props} />);
+  function renderTree(props) {
+    const { store } = dispatchSignInActions();
+
+    return findRenderedComponentWithType(renderIntoDocument(
+      <Provider store={store}>
+        <AuthenticateButtonBase i18n={getFakeI18nInst()} {...props} />
+      </Provider>
+    ), AuthenticateButtonBase);
+  }
 
   const render = (props) => findDOMNode(renderTree(props));
 
@@ -93,7 +100,7 @@ describe('<AuthenticateButton />', () => {
     };
     sinon.stub(config, 'get', (key) => _config[key]);
 
-    const { store } = createStore();
+    const { store } = dispatchSignInActions();
     store.dispatch(setAuthToken(userAuthToken({ user_id: 99 })));
     const apiConfig = { token: store.getState().api.token };
     expect(apiConfig.token).toBeTruthy();
@@ -107,7 +114,7 @@ describe('<AuthenticateButton />', () => {
   });
 
   it('pulls isAuthenticated from state', () => {
-    const { store } = createStore(combineReducers({ api }));
+    const { store } = dispatchAnonymousActions();
     expect(mapStateToProps(store.getState()).isAuthenticated).toEqual(false);
     store.dispatch(setAuthToken(userAuthToken({ user_id: 123 })));
     expect(mapStateToProps(store.getState()).isAuthenticated).toEqual(true);

From ce37eae24b48e310292aa4478a7cd7638796b899 Mon Sep 17 00:00:00 2001
From: Matthew Riley MacPherson <matt@lonelyvegan.com>
Date: Thu, 15 Jun 2017 19:35:25 +0100
Subject: [PATCH 2/2] chore: PR tweaks

---
 src/core/components/AuthenticateButton/index.js      | 4 ----
 tests/unit/amo/helpers.js                            | 4 ++--
 tests/unit/core/components/TestAuthenticateButton.js | 4 ++--
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/core/components/AuthenticateButton/index.js b/src/core/components/AuthenticateButton/index.js
index e1f261a9e3f..c1ce11a1d35 100644
--- a/src/core/components/AuthenticateButton/index.js
+++ b/src/core/components/AuthenticateButton/index.js
@@ -66,10 +66,6 @@ export class AuthenticateButtonBase extends React.Component {
     // appearance will transition to the hover state onClick when using a
     // mobile browser. This is the cause of
     // https://github.com/mozilla/addons-frontend/issues/1904
-    //
-    // We tried to use https://css-tricks.com/annoying-mobile-double-tap-link-issue/#article-header-id-2
-    // and media queries to fix this but the `href` solution worked so I'm
-    // going with this.
     return (
       <Button onClick={this.onClick} href="#" {...otherProps}>
         {noIcon ? null : <Icon name="user-dark" />}
diff --git a/tests/unit/amo/helpers.js b/tests/unit/amo/helpers.js
index 4443ec9ce19..25593a8c8e3 100644
--- a/tests/unit/amo/helpers.js
+++ b/tests/unit/amo/helpers.js
@@ -66,7 +66,7 @@ export const signedInApiState = Object.freeze({
   clientApp: 'firefox',
 });
 
-export function dispatchAnonymousActions({
+export function dispatchClientMetadata({
   store = createStore().store,
   clientApp = 'android',
   lang = 'en-US',
@@ -86,7 +86,7 @@ export function dispatchSignInActions({
   authToken = userAuthToken(),
   ...otherArgs
 } = {}) {
-  const { store } = dispatchAnonymousActions(otherArgs);
+  const { store } = dispatchClientMetadata(otherArgs);
 
   store.dispatch(setAuthToken(authToken));
 
diff --git a/tests/unit/core/components/TestAuthenticateButton.js b/tests/unit/core/components/TestAuthenticateButton.js
index 8a34cb531d7..b708aae840e 100644
--- a/tests/unit/core/components/TestAuthenticateButton.js
+++ b/tests/unit/core/components/TestAuthenticateButton.js
@@ -14,7 +14,7 @@ import {
   mapStateToProps,
 } from 'core/components/AuthenticateButton';
 import {
-  dispatchAnonymousActions,
+  dispatchClientMetadata,
   dispatchSignInActions,
 } from 'tests/unit/amo/helpers';
 import { getFakeI18nInst, userAuthToken } from 'tests/unit/helpers';
@@ -114,7 +114,7 @@ describe('<AuthenticateButton />', () => {
   });
 
   it('pulls isAuthenticated from state', () => {
-    const { store } = dispatchAnonymousActions();
+    const { store } = dispatchClientMetadata();
     expect(mapStateToProps(store.getState()).isAuthenticated).toEqual(false);
     store.dispatch(setAuthToken(userAuthToken({ user_id: 123 })));
     expect(mapStateToProps(store.getState()).isAuthenticated).toEqual(true);