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

Groups codes by pairs or threes depending on character count #216

Closed
wants to merge 5 commits into from

Conversation

beaucollins
Copy link
Collaborator

Purely an opinion but I find it easier to read two groups of three digits vs three groups of two digits.

And for 8 digit length codes I would say two groups of four digits would be better than four groups of two digits.

This change groups any code divisible by three into groups of three, divisible by two in pairs, otherwise no groupings.

img_1ee0ac437151-1

@mide
Copy link

mide commented Oct 27, 2017

That's a clever and elegant solution. 👍

@ghost
Copy link

ghost commented Nov 9, 2017

I much prefer this over the always-by-twos approach that currently exists.

@jansenfuller
Copy link

Is there a particular reason why this has been shelved?

@beaucollins
Copy link
Collaborator Author

@jansenfuller I think it just needs to be brought to the attention of @mattrubin

@codecov
Copy link

codecov bot commented Jan 3, 2018

Codecov Report

Merging #216 into develop will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #216      +/-   ##
===========================================
+ Coverage    40.21%   40.26%   +0.05%     
===========================================
  Files           36       36              
  Lines         2268     2270       +2     
===========================================
+ Hits           912      914       +2     
  Misses        1356     1356
Impacted Files Coverage Δ
Authenticator/Source/TokenRowModel.swift 96.15% <100%> (+0.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fc8946...01d65f0. Read the comment docs.

@mattrubin mattrubin added this to the 2.next milestone Apr 24, 2018
@mattrubin
Copy link
Owner

I'm sorry for the long delay in addressing this issue!

After much thought, I've opened a new PR which adds a settings screen that allows the user to choose whether they prefer passwords rendered in groups of two digits or groups of three. I'd love to hear feedback from all the folks who prefer groups of three on whether this new approach feels like a satisfactory solution.

@mattrubin
Copy link
Owner

This PR has been superseded by #290. Thank you to @beaucollins for this proposal!

@mattrubin mattrubin closed this Sep 26, 2018
@mattrubin mattrubin deleted the feature/digit-groups branch December 24, 2018 00:53
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.

None yet

4 participants