Skip to content

Stacks recovery key#974

Merged
el-mapache merged 1 commit intomasterfrom
ab-stacked-pkey
Jan 25, 2017
Merged

Stacks recovery key#974
el-mapache merged 1 commit intomasterfrom
ab-stacked-pkey

Conversation

@el-mapache
Copy link
Copy Markdown
Contributor

@el-mapache el-mapache commented Jan 20, 2017

Why: To make the code easier for the user to write down

Screenshot:

screen shot 2017-01-23 at 4 49 36 pm

@el-mapache
Copy link
Copy Markdown
Contributor Author

el-mapache commented Jan 20, 2017

@rtwell hello! Just missing a couple of icons for this ticket:

  • the key
  • the scissors

Also, something about the letter spacing in the screenshot seems weird. Should this screen be using a different font?

@jessieay
Copy link
Copy Markdown
Contributor

Might just be me, but when I see this it looks like a list of recovery keys rather than a single one, since I am used to other services giving me a list of recovery codes when signing up. Did we test this new design at all? Wondering if others would see the same thing.

@el-mapache el-mapache force-pushed the ab-stacked-pkey branch 2 times, most recently from f882311 to 8350ed3 Compare January 23, 2017 20:42
@rtwell
Copy link
Copy Markdown

rtwell commented Jan 23, 2017

@jessieay we're going to test this one! and the screen to enter your key looks like this:

image

@rtwell
Copy link
Copy Markdown

rtwell commented Jan 23, 2017

@el-mapache can we make the personal key type the Navy blue? #112E51

@el-mapache
Copy link
Copy Markdown
Contributor Author

@rtwell whoops, forgot to push that. It looks very dark on the text though

@rtwell
Copy link
Copy Markdown

rtwell commented Jan 23, 2017

nice and high contrast! love it. thanks!

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.

These color classes already exist with basscss, you can access them with .blue and .navy. We have all of these http://basscss.com/v7/docs/colors/ defined in stylesheets/variables/colors.

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.

Ah, I was trying to find the color classes, I thought they were ours, not basecss

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.

@hursey013 I removed the redundant class definitions and also an unnecessary color class in the markup

**Why**: To make the code easier for the user to write down
@@ -0,0 +1,14 @@
#recovery-code(class="col-12 border-box mt4 mb3 py2 px3 fs-20p sans-serif\
border border-dashed border-red rounded-xl relative")
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.

If we wind up using this pattern anywhere else we can probably roll some of this up into a new class, but fine for now.

Copy link
Copy Markdown
Contributor

@hursey013 hursey013 left a comment

Choose a reason for hiding this comment

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

lgtm!

@el-mapache el-mapache merged commit dbfb4e8 into master Jan 25, 2017
@el-mapache el-mapache deleted the ab-stacked-pkey branch January 25, 2017 17:13
amoose pushed a commit that referenced this pull request Mar 7, 2017
**Why**: To make the code easier for the user to write down
amoose pushed a commit that referenced this pull request Mar 8, 2017
**Why**: To make the code easier for the user to write down
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants