Sign-in button is now disabled when token is empty or whitespace

lib/sign-in-component.js

@@ -7,6 +7,12 @@ class SignInComponent {
   constructor (props) {
     this.props = props
     etch.initialize(this)
+
+    this.refs.editor.onDidChange(() => {
+      const token = this.refs.editor.getText().trim()
+      this.refs.loginButton.disabled = !token
+    })
+
     this.disposables = new CompositeDisposable()
     this.disposables.add(this.props.authenticationProvider.onDidChange(() => {
       etch.update(this)
@@ -23,25 +29,33 @@ class SignInComponent {
 
   update (props) {
     Object.assign(this.props, props)
-    etch.update(this)
+    return etch.update(this)
   }
 
   render () {
     return $.div({className: 'SignInComponent', tabIndex: -1},
       $.span({className: 'SignInComponent-GitHubLogo'}),
       $.h3(null, 'Sign in with GitHub'),
-      this.props.authenticationProvider.isSigningIn()
-        ? this.renderSigningInIndicator()
-        : this.renderTokenPrompt()
+      this.renderSigningInIndicator(),
+      this.renderTokenPrompt()
     )
   }
 
   renderSigningInIndicator () {
-    return $.span({className: 'loading loading-spinner-tiny inline-block'})
+    let props = {}
+    if (this.props.authenticationProvider.isSigningIn()) {
+      props.className = 'loading loading-spinner-tiny inline-block'
+    } else {
+      props.style = {display: 'none'}
+    }
+
+    return $.span(props)
   }
 
   renderTokenPrompt () {
-    return $.div(null,
+    const props = this.props.authenticationProvider.isSigningIn() ? {style: {display: 'none'}} : null
+
+    return $.div(props,
       $.p(null,
         'Visit ',
         $.a({href: 'https://teletype.atom.io/login', className: 'text-info'}, 'teletype.atom.io/login'),
@@ -56,7 +70,8 @@ class SignInComponent {
             ref: 'loginButton',
             type: 'button',
             className: 'btn btn-primary btn-sm inline-block-tight',
-            onClick: this.signIn
+            onClick: this.signIn,
+            disabled: true
           },
           'Sign in'
         )
@@ -66,15 +81,20 @@ class SignInComponent {
 
   renderErrorMessage () {
     return this.props.invalidToken
-      ? $.p({className: 'error-messages'}, 'That token does not appear to be valid.')
+      ? $.p({ref: 'errorMessage', className: 'error-messages'}, 'That token does not appear to be valid.')
       : null
   }
 
   async signIn () {
-    await this.update({invalidToken: false})
+    const {editor} = this.refs
+    const token = editor.getText().trim()
+    const signedIn = token ? await this.props.authenticationProvider.signIn(token) : false
 
-    const token = this.refs.editor.getText()
-    const signedIn = await this.props.authenticationProvider.signIn(token)
-    if (!signedIn) await this.update({invalidToken: true})
+    if (signedIn) {
+      await this.update({invalidToken: false})
+    } else {
+      editor.setText('')
+      await this.update({invalidToken: true})
+    }
   }
 }

test/helpers/fake-authentication-provider.js

@@ -0,0 +1,24 @@
+const {Disposable} = require('atom')
+
+module.exports =
+class FakeAuthenticationProvider {
+  constructor ({notificationManager}) {
+    this.notificationManager = notificationManager
+  }
+
+  onDidChange (callback) {
+    return new Disposable(callback)
+  }
+
+  async signIn (token) {
+    return true
+  }
+
+  isSigningIn () {
+    return false
+  }
+
+  async signOut () {}
+
+  dispose () {}
+}

test/helpers/fake-command-registry.js

@@ -0,0 +1,17 @@
+module.exports =
+class FakeCommandRegistry {
+  constructor () {
+    this.items = new Set()
+  }
+
+  add (elem, commands) {
+    this.items.add({ elem, commands })
+    return this
+  }
+
+  dispose () {
+    this.items.forEach(({elem, command}) => {
+      elem.dispose()
+    })
+  }
+}

test/helpers/fake-notification-manager.js

@@ -0,0 +1,14 @@
+module.exports =
+class FakeNotificationManager {
+  constructor () {
+    this.errorCount = 0
+  }
+
+  addInfo () {}
+
+  addSuccess () {}
+
+  addError () {
+    this.errorCount++
+  }
+}

test/helpers/fake-workspace.js

@@ -0,0 +1,34 @@
+const {Disposable} = require('atom')
+
+module.exports =
+class FakeWorkspace {
+  async open () {}
+
+  getCenter () {
+    return {
+      paneContainer: {
+        getElement () {
+          return document.createElement('div')
+        }
+      }
+    }
+  }
+
+  getElement () {
+    return document.createElement('div')
+  }
+
+  observeActiveTextEditor () {
+    return new Disposable(() => {})
+  }
+
+  onDidDestroyPaneItem () {
+    return new Disposable(() => {})
+  }
+
+  onDidChangeActivePaneItem () {
+    return new Disposable(() => {})
+  }
+
+  paneForItem () {}
+}

test/portal-list-component.test.js

@@ -1,11 +1,13 @@
 const assert = require('assert')
 const condition = require('./helpers/condition')
-const {Disposable} = require('atom')
 const FakeClipboard = require('./helpers/fake-clipboard')
 const {TeletypeClient} = require('@atom/teletype-client')
 const {startTestServer} = require('@atom/teletype-server')
 const PortalBindingManager = require('../lib/portal-binding-manager')
 const PortalListComponent = require('../lib/portal-list-component')
+const FakeNotificationManager = require('./helpers/fake-notification-manager')
+const FakeWorkspace = require('./helpers/fake-workspace')
+const FakeCommandRegistry = require('./helpers/fake-command-registry')
 
 suite('PortalListComponent', function () {
   if (process.env.CI) this.timeout(process.env.TEST_TIMEOUT_IN_MS)
@@ -243,53 +245,3 @@ suite('PortalListComponent', function () {
     return portalBindingManager
   }
 })
-
-class FakeWorkspace {
-  async open () {}
-
-  getCenter () {
-    return {
-      paneContainer: {
-        getElement () {
-          return document.createElement('div')
-        }
-      }
-    }
-  }
-
-  getElement () {
-    return document.createElement('div')
-  }
-
-  observeActiveTextEditor () {
-    return new Disposable(() => {})
-  }
-
-  onDidDestroyPaneItem () {
-    return new Disposable(() => {})
-  }
-
-  onDidChangeActivePaneItem () {
-    return new Disposable(() => {})
-  }
-
-  paneForItem () {}
-}
-
-class FakeNotificationManager {
-  constructor () {
-    this.errorCount = 0
-  }
-
-  addInfo () {}
-
-  addSuccess () {}
-
-  addError () {
-    this.errorCount++
-  }
-}
-
-class FakeCommandRegistry {
-  add () { return new Set() }
-}

test/sign-in-component.test.js

@@ -0,0 +1,79 @@
+const assert = require('assert')
+const FakeAuthenticationProvider = require('./helpers/fake-authentication-provider')
+const SignInComponent = require('../lib/sign-in-component')
+const FakeNotificationManager = require('./helpers/fake-notification-manager')
+const FakeCommandRegistry = require('./helpers/fake-command-registry')
+
+suite('SignInComponent', function () {
+  test('has correct fields', () => {
+    const component = buildComponent()
+    assert(component.refs.editor)
+    assert(component.refs.loginButton)
+    assert(!component.refs.errorMessage)
+  })
+
+  test('disables button when empty token specified', () => {
+    {
+      // It should be disabled by default
+      const component = buildComponent()
+      assert(component.refs.loginButton.disabled)
+    }
+
+    {
+      // Whitespace should also leave the button disabled
+      const component = buildComponent()
+      component.refs.editor.setText('    ')
+      assert(component.refs.loginButton.disabled)
+    }
+
+    {
+      // It should be disabled when set to an empty string
+      const component = buildComponent()
+      component.refs.editor.setText('')
+      assert(component.refs.loginButton.disabled)
+    }
+  })
+
+  test('enables button when non-empty token specified', () => {
+    const component = buildComponent()
+    component.refs.editor.setText('some-token')
+    assert(!component.refs.loginButton.disabled)
+  })
+
+  test('reports errors attempting to sign in', async () => {
+    {
+      const component = buildComponent()
+      const {authenticationProvider} = component.props
+      const notifications = authenticationProvider.notificationManager
+
+      authenticationProvider.signIn = (token) => {
+        notifications.addError()
+        return false
+      }
+      component.refs.editor.setText('some-token')
+      await component.signIn()
+
+      // It should display an error message when the login attempt fails
+      assert.equal(notifications.errorCount, 1)
+    }
+
+    {
+      const component = buildComponent()
+
+      await component.signIn()
+
+      // It should show an error message about an invalid token
+      assert(component.refs.errorMessage)
+      assert.equal(component.refs.errorMessage.innerHTML, 'That token does not appear to be valid.')
+    }
+  })
+
+  function buildComponent () {
+    return new SignInComponent({
+      commandRegistry: new FakeCommandRegistry(),
+      authenticationProvider: new FakeAuthenticationProvider({
+        notificationManager: new FakeNotificationManager()
+      })
+    })
+  }
+})

test/teletype-package.test.js

@@ -267,7 +267,8 @@ suite('TeletypePackage', function () {
     popoverComponent.refs.signInComponent.signIn()
     await condition(() => (
       popoverComponent.refs.signInComponent.props.invalidToken &&
-      popoverComponent.refs.signInComponent.refs.editor
+      popoverComponent.refs.signInComponent.refs.editor.getText() === '' &&
+      popoverComponent.refs.signInComponent.refs.loginButton.disabled
     ))
     assert.equal(await pack.credentialCache.get('oauth-token'), null)
     assert(!env.workspace.element.classList.contains('teletype-Authenticated'))
@@ -1094,6 +1095,21 @@ suite('TeletypePackage', function () {
     }
   })
 
+  test('client connection errors', async () => {
+    const env = buildAtomEnvironment()
+    const pack = await buildPackage(env)
+    await pack.sharePortal()
+    env.notifications.clear()
+
+    pack.client.emitter.emit('connection-error', new ErrorEvent('error', {message: 'connection-error'}))
+    assert.equal(env.notifications.getNotifications().length, 1)
+    const {type, message, options} = env.notifications.getNotifications()[0]
+    const {description} = options
+    assert.equal(type, 'error')
+    assert.equal(message, 'Connection Error')
+    assert(description.includes('connection-error'))
+  })
+
   test('reports errors attempting to sign in', async () => {
     const env = buildAtomEnvironment()
     const pack = await buildPackage(env, {signIn: false})
@@ -1114,21 +1130,6 @@ suite('TeletypePackage', function () {
     assert(description.includes('some error'))
   })
 
-  test('client connection errors', async () => {
-    const env = buildAtomEnvironment()
-    const pack = await buildPackage(env)
-    await pack.sharePortal()
-    env.notifications.clear()
-
-    pack.client.emitter.emit('connection-error', new ErrorEvent('error', {message: 'connection-error'}))
-    assert.equal(env.notifications.getNotifications().length, 1)
-    const {type, message, options} = env.notifications.getNotifications()[0]
-    const {description} = options
-    assert.equal(type, 'error')
-    assert.equal(message, 'Connection Error')
-    assert(description.includes('connection-error'))
-  })
-
   let nextTokenId = 0
   async function buildPackage (env, options = {}) {
     const credentialCache = new FakeCredentialCache()