rodrigoprimo / social-connect

WordPress plugin for signup/login using social media providers.
http://wordpress.org/extend/plugins/social-connect/
GNU General Public License v3.0
73 stars 40 forks source link

What I did: #2

Closed Wirone closed 13 years ago

Wirone commented 13 years ago
Wirone commented 13 years ago

I made sc_render_login_form_social_connect($display_label = true) configurable, so when passed arg is false it doesn't echo the label, but only buttons. It's helpful in widget, which I also made (changed). Widget is configurable too, so we can set title and optional text/html before and after widget content (buttons).

I also changed every div with redirect_url attribute, because it wasn't accepted by W3C validator, to hidden input and connect.js functions now gets its value.

ps. There are so many additions/deletions because I changed folding locally (spaces for tabs), but now I see GitHub changed it back... tab = 2 spaces? Nevermind, please look at widget and other "real" changes.

Cheers, Wirone

thenbrent commented 13 years ago

Great thanks Wirone. I'll review all the changes in the next few days and merge them for the next public release. Thanks for your contributions, greatly appreciated. :)

thenbrent commented 13 years ago

Will the input element used in the login form uri function validate? It's not in a form element so looks uncompliant to me. https://github.com/thenbrent/social-connect/commit/33aab9c884ebc451da34ae040678023c41d66859#L4R165

I've also changed the parameters for the render_login_form function back to one parameter and built in a much better way to deal with "NULL" than having a "dead" parameter.

You can see the full merge here: https://github.com/thenbrent/social-connect/commit/017aa543a99b3b2ecc9916e634dcbe232b4a2b33

Wirone commented 13 years ago

Will the input element used in the login form uri function validate? It's not in a form element so looks uncompliant to me.

As you can see here:

<input> (and <textarea>, <button>) is an inline element, like <span> or <em>, so can be used in any context where inline content is allowed, which is virtually everywhere.

Why? Because otherwise <input> couldn't be contained in, for example, a paragraph, even if that paragraph was itself within a form,

and

Besides what Toby said about why it's valid (which is correct) it's also reasonable if you're using controls to receive user input that is going to be used on the client side rather than for submitting to a server. An example would be a client-side measurement or currency converter coded in Javascript. Functionally speaking there's no need for a form since nothing is being submitted to the server.

So it's best solution for keeping data such as redirect_url :)

I have also added some comments to your commits, so please check them between whiles.

I'm really glad that I could (and still can) help :)

thenbrent commented 13 years ago

Great, thanks for the link on <input> standards. Very useful to know.

I've made appropriate fixes for your other comments too.

Is it ok for your purposes to have the single parameter on sc_render_login_form_social_connect function?

If so, I'll finish the merge and deploy a new public release soon.

Wirone commented 13 years ago

If it works as it should it's ok for me of course. I run yesterday your developed version and widget was rendered properly, buttons with label at login form too, so it should work fine :)

Wirone commented 13 years ago

I see you are working on new release, maybe take an eye on my 2 commits? Especially on translated admin page :)

thenbrent commented 13 years ago

Great, thanks again. I've merged these (great) changes and will be making a release later tonight or tomorrow. Let me know if there is anything else to add before I do. :)

Wirone commented 13 years ago

I will not have time today for working on it.. I think it's ready for new release and we can think about next changes and improvements ;)

I'm thinking only about changing link in readme, because my Wordpress profile isn't interesting ;) I have newly created blog about programming etc. (powered by Wordpress ;)), but it's in polish. Currently I don't have any portfolio in english. I wanted to make this blog multilanguage, but I didn't find good plugin for it yet.

thenbrent commented 13 years ago

I've just changed the link to your blog and will release soon. :)

On Thu, Apr 28, 2011 at 3:48 PM, Wirone < reply@reply.github.com>wrote:

I will not have time today for working on it.. I think it's ready for new release and we can think about next changes and improvements ;)

I'm thinking only about changing link in readme, because my Wordpress profile isn't interesting ;) I have newly created blog about programming etc. (powered by Wordpress ;)), but it's in polish. Currently I don't have any portfolio in english. I wanted to make this blog multilanguage, but I didn't find good plugin for it yet.

Reply to this email directly or view it on GitHub: https://github.com/thenbrent/social-connect/pull/2#issuecomment-1068863