Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add i18n support #65

Merged
merged 4 commits into from Oct 4, 2013

Conversation

Projects
None yet
2 participants
Contributor

guilhermesimoes commented Aug 5, 2013

First, I refactored the request_phase function since it was getting kinda messy 😀

After that, adding i18n support was as simple as adding lang to an array. As you can see, this way it's really easy to add support for new features.

As for the i18n feature, it allows you to do this:

<%= link_to 'Twitter', "/auth/twitter?lang=#{I18n.locale}" %>

guilhermesimoes added some commits Aug 3, 2013

@guilhermesimoes guilhermesimoes Refactor request_phase method.
The request params are already accessible in request.params so there is no
need to get them from the session (which is set after the request by omniauth).
https://github.com/intridea/omniauth/blob/deea8f23642734217fc8a91cfe9a09a69595d638/lib/omniauth/strategy.rb#L200

There is also no need to conditionally initialize options[:authorize_params]
and options[:request_params] as these are already set automatically by
omniauth-oauth.
https://github.com/intridea/omniauth-oauth/blob/43008e42b257d9b7c0b98122cf03ce14b021d782/lib/omniauth/strategies/oauth.rb#L16

This refactor will make it really easy to add new params in the future.
3f8a930
@guilhermesimoes guilhermesimoes Refactor spec.
We should stub the request and not the session (which is set after the
request by omniauth)
https://github.com/intridea/omniauth/blob/deea8f23642734217fc8a91cfe9a09a69595d638/lib/omniauth/strategy.rb#L200

It also doesn't make sense to set options[:request_params] to nil when
omniauth-oauth is already initializing it as an empty hash
https://github.com/intridea/omniauth-oauth/blob/43008e42b257d9b7c0b98122cf03ce14b021d782/lib/omniauth/strategies/oauth.rb#L16
991a801
@guilhermesimoes guilhermesimoes Add i18n support.
We can now pass the `lang` param to the authorization url to dynamically
set the language of the Twitter prompt:
/auth/twitter?lang=pt

The default language of the prompt could already be set in the OmniAuth
config with:
:authorize_params => { :lang => 'pt' }

Fix #45
c4d6570
@guilhermesimoes guilhermesimoes Refactor image_url method.
Since options is a class variable, there's no need to pass it as an
argument.
aa43b89
Contributor

guilhermesimoes commented Oct 4, 2013

@arunagw could you please take a look at this? Thanks!

@arunagw arunagw added a commit that referenced this pull request Oct 4, 2013

@arunagw arunagw Merge pull request #65 from GuilhermeSimoes/i18n
Add i18n support
ece9eb9

@arunagw arunagw merged commit ece9eb9 into arunagw:master Oct 4, 2013

1 check passed

default The Travis CI build passed
Details
Owner

arunagw commented Oct 4, 2013

Looks good! Thanks

And sorry for being too late here.

Contributor

guilhermesimoes commented Oct 5, 2013

No problem. Thanks again!

@guilhermesimoes guilhermesimoes deleted the unknown repository branch Nov 11, 2013

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