Skip to content

Adds authenticator app tooltip#904

Merged
el-mapache merged 2 commits intomasterfrom
ab-authenticator-tooltip
Jan 29, 2017
Merged

Adds authenticator app tooltip#904
el-mapache merged 2 commits intomasterfrom
ab-authenticator-tooltip

Conversation

@el-mapache
Copy link
Copy Markdown
Contributor

@el-mapache el-mapache commented Dec 28, 2016

Why: To ensure users understand what the authenticator app is, and how they can
use it

EDIT: new visuals

screen shot 2016-12-28 at 5 03 20 pm

screen shot 2016-12-28 at 5 04 08 pm

@el-mapache el-mapache force-pushed the ab-authenticator-tooltip branch from bacd42b to c41e9ac Compare December 28, 2016 14:07
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should authentication application be authenticator app to match the main text on the screen?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will ask what copy thinks!

Copy link
Copy Markdown
Contributor

@monfresh monfresh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % comment

@el-mapache el-mapache force-pushed the ab-authenticator-tooltip branch from c41e9ac to 4605b7c Compare December 28, 2016 22:00
@el-mapache
Copy link
Copy Markdown
Contributor Author

el-mapache commented Dec 28, 2016

I believe I have amended the wording to authentication in all the files that I touched. I think i've looked at it too many times, it is starting to look really funny to me...

@el-mapache
Copy link
Copy Markdown
Contributor Author

I also changed the heading from authenticator to authentication

@monfresh
Copy link
Copy Markdown
Contributor

Looks good, although I just noticed something else. Is it OK for us to name Google Authenticator and Authy? Will that make it look like we endorse those products?

@el-mapache
Copy link
Copy Markdown
Contributor Author

el-mapache commented Dec 29, 2016

We would have to ask about that as well, I'm not sure. I suppose it depends on if providing examples can be construed as an endorsement. I guess I wouldn't be too surprised if there were a rule against it...

@hursey013
Copy link
Copy Markdown
Contributor

I wonder if rather than having the tooltip icon sort of squeezed in between sentences if we could just make authentication app a link that triggers the tooltip? I think that may look more natural. What do you think?

@el-mapache
Copy link
Copy Markdown
Contributor Author

el-mapache commented Dec 29, 2016

I personally don't like it there, but we don't have a consistent look for these (some places use the ? some use links). If we want to just start removing these bubbles and using links I am down.

@rtwell
Copy link
Copy Markdown

rtwell commented Dec 29, 2016

hey gang! i recently stumbled across this issue and i'd love to put a little polish on the design of this tooltip—there is a disconnect between the tooltip ? and the what it's referencing. @el-mapache @monfresh

@el-mapache
Copy link
Copy Markdown
Contributor Author

Blocked sounds harsh, just waiting on further comment from ryan 😄

@rtwell
Copy link
Copy Markdown

rtwell commented Jan 3, 2017 via email

@jessieay
Copy link
Copy Markdown
Contributor

Hello @rtwell -- do you have time to polish this off? Seems like the PR is getting a bit stale

@rtwell
Copy link
Copy Markdown

rtwell commented Jan 19, 2017

thanks for the reminder @jessieay
i think we could just reword the text so that the tooltip comes at the end of the paragraph @andrewhughey @RyanSibley @jeanninehunter thoughts?

@jeanninehunter
Copy link
Copy Markdown
Contributor

@rtwell @RyanSibley @andrewhughey @esgoodman : Agree with either hyperlinking authenticator app or superimposing the question mark and making it slightly smaller so it doesn't break the sentences. Also, for the definition itself, rereading #1233, makes me wonder if we should identify a specific tool or company such as Google and Authy. Perhaps the definition itself should read as: "An authentication application is a mobile security application that generates sign-in passcodes even if you don't have an Internet connection or mobile service."

@esgoodman
Copy link
Copy Markdown
Contributor

esgoodman commented Jan 21, 2017

@jeanninehunter @el-mapache We should just close this PR so we can all move on, I think. My request: merge this after

  1. Moving the tooltip as-is to the end of the sentence to avoid that unsightly white space.

  2. Changing the text to: An authentication application is a mobile security app that generates security codes even if you don't have an Internet connection or cellular service. (Changed text slightly to conform to the new security code vocabulary and to make it clear that there is the Internet and there is cellular service).

Then we create new issues to address:

  1. Visual design of the tooltip: @rtwell just put some visual polish on it and the new comps are almost ready. He will be uploading them next week, but that refinement is separate from the existence of the tooltip.

  2. Whether we mention Google Authenticator and/or Authy, now currently being discussed in comments to Promotion of current master to stages/qa #1182. Those comments have taken so long to resolve that it seems to indicate that the question should just have its own issue. 👈 @jkim1217.

  3. Rules about using tooltips: We created a tooltip icon because it seemed like we'd need a lot of tooltip-style inline explanation. However, we now two tooltips right now in the entire account creation/authentication flow, so that assumption seems to be false. I'm open to another solutions but don't want to drag out this PR.

@andrewhughey
Copy link
Copy Markdown
Contributor

I'm all for @esgoodman's proposal above. Expect new issues to resolve tooltip design, naming an auth app, and guidelines for tooltips.

@monfresh
Copy link
Copy Markdown
Contributor

What's the status of this PR? Have Liz's changes been incorporated? Also, it looks like it need a rebase with master.

@el-mapache
Copy link
Copy Markdown
Contributor Author

@monfresh Yes, I did most of the work yesterday. Just cleaning up the specs

@monfresh
Copy link
Copy Markdown
Contributor

Cool. I couldn't tell because there's only 1 commit 😄

@el-mapache el-mapache force-pushed the ab-authenticator-tooltip branch 3 times, most recently from 4f4d565 to 26ddde4 Compare January 28, 2017 19:54
**Why**: To ensure users understand what the app is, and how they can
use it
**Why**: It is the easiest way to allow the presenter to have access to
mehtods inside view helpers
@el-mapache el-mapache force-pushed the ab-authenticator-tooltip branch from 26ddde4 to 03746f3 Compare January 29, 2017 00:51
@el-mapache el-mapache merged commit ff2eb1b into master Jan 29, 2017
@el-mapache el-mapache deleted the ab-authenticator-tooltip branch January 29, 2017 00:59
amoose pushed a commit that referenced this pull request Mar 7, 2017
* Adds authenticator app tooltip

**Why**: To ensure users understand what the app is, and how they can
use it

* Pass optional explicit view context to presenter

**Why**: It is the easiest way to allow the presenter to have access to
mehtods inside view helpers
amoose pushed a commit that referenced this pull request Mar 8, 2017
* Adds authenticator app tooltip

**Why**: To ensure users understand what the app is, and how they can
use it

* Pass optional explicit view context to presenter

**Why**: It is the easiest way to allow the presenter to have access to
mehtods inside view helpers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants