Skip to content

issue 19 #169

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.

Sign up for GitHub

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

Open
sebrittain wants to merge 7 commits into
base: master
Choose a base branch
from
Open

issue 19 #169

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b5ca790
issue 19
sebrittain Jul 10, 2012
1861b7e
fixed error where user could undo correct password
sebrittain Jul 11, 2012
e7a129d
fixed checkbox issue
sebrittain Jul 19, 2012
24eee4c
fixed username mistake in user.rb
sebrittain Jul 19, 2012
26a337b
making code better
sebrittain Jul 24, 2012
4036651
abstracted function working but keyup not working the way it should
sebrittain Jul 24, 2012
bea7a5e
password works correctly and doesn't repeat code
sebrittain Jul 25, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added app/assets/images/accept.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added app/assets/images/cross.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
52 changes: 52 additions & 0 deletions app/assets/stylesheets/quadbase.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1302,3 +1302,55 @@ a.tag_cloud_4 { font-size: 20px; line-height:4;}
padding-top: 4px;
width: 100%;
}
u1, li {
margin:0;
padding:0;
list-style-type:none;
}
.invalid {

background:url('/assets/cross.png') no-repeat 0 50%;
padding-left:22px;
line-height:24px;
color:#ec3f41;
}
.valid{
background:url("/assets/accept.png") no-repeat 0 50%;
padding-left:22px;
line-height:24px;
color:#3a7d34;
}

#pswd_info {
position:absolute;
bottom:55px;
bottom: -115px\9;
left:330px;
width:250px;
padding:15px;
background:#fefefe;
font-size:.875em;
border-radius:5px;
box-shadow:0 1px 3px #ccc;
border:1px solid #ddd;
}
#pswd_info h4 {
margin: 0 0 10px 0;
padding:0;
font-weight:normal;
}
#pswd_info::before {
content: "\25C4";
position:absolute;
top:57px;
left:-4%;
font-size:16px;
line-height:14px;
color:#ddd;
text-shadow:none;
display:block;
}
#pswd_info {
display:none;
}

2 changes: 1 addition & 1 deletion app/models/user.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class User < ActiveRecord::Base

validates_presence_of :first_name, :last_name, :username, :user_profile
validates_uniqueness_of :username, :case_sensitive => false
validates_length_of :username, :in => 3..40
validates_length_of :username, :in => 8..40

@Dantemss Dantemss Jul 19, 2012

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we want to enforce this. Short usernames are probably fine.

Just enforce that the password has at least 8 characters.

You could write a custom validation to ensure that the password also has at least 1 letter and 1 number, but that's probably not necessary.

@sebrittain sebrittain Jul 19, 2012

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.

oops thought that variable was :password, not :username, fixed now

validates_format_of :username, :with => /^[A-Za-z\d_]+$/ # alphanum + _
validate :validate_username_unchanged, :on => :update

Expand Down
67 changes: 63 additions & 4 deletions app/views/devise/registrations/new.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
<%= f.email_field :email %></p>
<p><b>Password</b><br/>
<%= f.password_field :password %></p>
<div id="pswd_info" >
<h4>Password must meet the following requirements:</h4>
<u1>
<li id ="letter" class="invalid">At least <strong>one letter</strong></li>
<li id="number" class ="invalid">At least <strong>one number</strong></li>
<li id="length" class="invalid">At least <strong>8 characters</strong></li>
</u1>
</div>
<p><b>Password Confirmation</b><br/>
<%= f.password_field :password_confirmation %></p>

Expand All @@ -41,15 +49,66 @@
<%# JS to enable/disable the submit button dependent on if the agree checkbox is checked %>
<% content_for :javascript do %>
<%= javascript_tag do %>
var len = false;
var letter = false;
var number = false;
$('#agreement_checkbox').live('click', function() {
if ($('#agreement_checkbox').is(':checked')) {
$('#register_submit').removeAttr('disabled');
$('#register_submit').removeClass('ui-state-disabled');
}
if ($('#agreement_checkbox').is(':checked') & (len == true)& (letter == true) & (number == true)){

@Dantemss Dantemss Jul 19, 2012

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you abstract this if statement and the contents into some JS function, to avoid repeating the code. Just call it checkPassword(event) or something like that (don't worry about the event variable), then you can probably call it like: $('#agreement_checkbox').on('click', checkPassword)

$('#register_submit').removeAttr('disabled');
$('#register_submit').removeClass('ui-state-disabled');

}
else {
$('#register_submit').attr('disabled', 'disabled');
$('#register_submit').addClass('ui-state-disabled');
}
});

$('#password_field').focus(function(){
if ($('#agreement_checkbox').is(':checked') & (len == true)& (letter == true) & (number == true)){
$('#register_submit').removeAttr('disabled');
$('#register_submit').removeClass('ui-state-disabled');

}
else {
$('#register_submit').attr('disabled', 'disabled');
$('#register_submit').addClass('ui-state-disabled');
}
});

$('input[type=password]').keyup(function(){

@Dantemss Dantemss Jul 19, 2012

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You might want to make this affect only the main password field, and add a separate check for the confirmation matching. Of course, the other check should be triggered when typing to both the main field and the confirmation field.

//set password variable
var pswd = $(this).val();

//validate the length
if (pswd.length < 8) {
$('#length').removeClass('valid').addClass('invalid');
len = false;
} else {
$('#length').removeClass('invalid').addClass('valid');
len = true;
}
//validate letter
if (pswd.match(/[A-z]/)){
$('#letter').removeClass('invalid').addClass('valid');
letter = true;
} else {
$('#letter').removeClass('valid').addClass('invalid');
letter = false;
}
//validate number
if (pswd.match(/\d/)){
$('#number').removeClass('invalid').addClass('valid');
number = true;
} else {
$('#number').removeClass('valid').addClass('invalid');
number = false;
}

@Dantemss Dantemss Jul 19, 2012

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice. Can you add one more check that is valid if the password and the confirmation match?

}).focus(function(){
$('#pswd_info').show();
}).blur(function(){
$('#pswd_info').hide();
});
<% end %>
<% end %>