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

Profile Editor #617

Merged
merged 37 commits into from Dec 24, 2012

Conversation

Projects
None yet
3 participants
@sunnyratilal
Member

sunnyratilal commented Dec 23, 2012

Profile editor completed.

Issue #503

@pippinsplugins

This comment has been minimized.

Show comment
Hide comment
@pippinsplugins

pippinsplugins Dec 23, 2012

Member

My notes above still apply, but I want to change this a bit. There should only be one form, not one each each name, email, password. The single form will contain all the fields, otherwise it is simply too convoluted for site admins to setup.

Member

pippinsplugins commented Dec 23, 2012

My notes above still apply, but I want to change this a bit. There should only be one form, not one each each name, email, password. The single form will contain all the fields, otherwise it is simply too convoluted for site admins to setup.

@sunnyratilal

This comment has been minimized.

Show comment
Hide comment
@sunnyratilal

sunnyratilal Dec 23, 2012

Member

Should I put everything into one shortcode then? [edd_profile_editor]?

Member

sunnyratilal commented Dec 23, 2012

Should I put everything into one shortcode then? [edd_profile_editor]?

@pippinsplugins

This comment has been minimized.

Show comment
Hide comment
@pippinsplugins

pippinsplugins Dec 23, 2012

Member

Yes please.

Member

pippinsplugins commented Dec 23, 2012

Yes please.

@sunnyratilal

This comment has been minimized.

Show comment
Hide comment
@sunnyratilal

sunnyratilal Dec 23, 2012

Member

@pippinsplugins Think I've got everything.

Member

sunnyratilal commented Dec 23, 2012

@pippinsplugins Think I've got everything.

@sunnyratilal

This comment has been minimized.

Show comment
Hide comment
@sunnyratilal

sunnyratilal Dec 23, 2012

Member

I've had to go back to using wp_set_password() because if the user doesn't wish to change the password and a blank password is set, WordPress sets the password to some random string - what happened when I just tested it and I had to change my password manually in the database to log back in again.

Member

sunnyratilal commented Dec 23, 2012

I've had to go back to using wp_set_password() because if the user doesn't wish to change the password and a blank password is set, WordPress sets the password to some random string - what happened when I just tested it and I had to change my password manually in the database to log back in again.

Sunny Ratilal added some commits Dec 23, 2012

Sunny Ratilal
Added message for users if they want to change their password because…
… WordPress deletes the user's session after they change their password
@sunnyratilal

This comment has been minimized.

Show comment
Hide comment
@sunnyratilal

sunnyratilal Dec 23, 2012

Member

Covered all your notes.

Member

sunnyratilal commented Dec 23, 2012

Covered all your notes.

@spencerfinnell

This comment has been minimized.

Show comment
Hide comment
@spencerfinnell

spencerfinnell Dec 23, 2012

Member

The form should just post to itself by leaving the action blank: http://markjaquith.wordpress.com/2009/09/21/php-server-vars-not-safe-in-forms-or-links/

Also, shouldn't edd_get_current_page_url() escape the output inside the function instead of having to do it every time it's called somewhere else?

Member

spencerfinnell commented Dec 23, 2012

The form should just post to itself by leaving the action blank: http://markjaquith.wordpress.com/2009/09/21/php-server-vars-not-safe-in-forms-or-links/

Also, shouldn't edd_get_current_page_url() escape the output inside the function instead of having to do it every time it's called somewhere else?

@pippinsplugins

This comment has been minimized.

Show comment
Hide comment
@pippinsplugins

pippinsplugins Dec 23, 2012

Member

@spencerfinnell Leaving the action attribute blank causes validation errors in some browsers (IE I think). As long as the URL is escaped, then it should be fine to use.

And yes, I agree, edd_get_current_page_url() should esc its return.

Member

pippinsplugins commented Dec 23, 2012

@spencerfinnell Leaving the action attribute blank causes validation errors in some browsers (IE I think). As long as the URL is escaped, then it should be fine to use.

And yes, I agree, edd_get_current_page_url() should esc its return.

@sunnyratilal

This comment has been minimized.

Show comment
Hide comment
@sunnyratilal

sunnyratilal Dec 24, 2012

Member

Question: should I get rid of the <legend> for Change Your Display Name and Change your Email Address to avoid repetition because the fields directly underneath have a <label> which say pretty much the same?

http://cl.ly/image/0J3e12471w3C

Member

sunnyratilal commented Dec 24, 2012

Question: should I get rid of the <legend> for Change Your Display Name and Change your Email Address to avoid repetition because the fields directly underneath have a <label> which say pretty much the same?

http://cl.ly/image/0J3e12471w3C

@pippinsplugins

This comment has been minimized.

Show comment
Hide comment
@pippinsplugins
Member

pippinsplugins commented Dec 24, 2012

Yes.

@sunnyratilal

This comment has been minimized.

Show comment
Hide comment
@sunnyratilal

sunnyratilal Dec 24, 2012

Member

@pippinsplugins Is this ready to merge or is there still some stuff left to do?

Member

sunnyratilal commented Dec 24, 2012

@pippinsplugins Is this ready to merge or is there still some stuff left to do?

@pippinsplugins

This comment has been minimized.

Show comment
Hide comment
@pippinsplugins

pippinsplugins Dec 24, 2012

Member

I will give it a test and then let you know.

Member

pippinsplugins commented Dec 24, 2012

I will give it a test and then let you know.

@sunnyratilal

This comment has been minimized.

Show comment
Hide comment
@sunnyratilal

sunnyratilal Dec 24, 2012

Member

Thanks.

Member

sunnyratilal commented Dec 24, 2012

Thanks.

@pippinsplugins pippinsplugins merged commit de60640 into easydigitaldownloads:master Dec 24, 2012

@pippinsplugins

This comment has been minimized.

Show comment
Hide comment
@pippinsplugins

pippinsplugins Dec 24, 2012

Member

Merged.

Member

pippinsplugins commented Dec 24, 2012

Merged.

@sunnyratilal sunnyratilal deleted the sunnyratilal:profile-editor-dev branch Dec 25, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment