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

Store helper proxies instead of raw view contexts #438

Merged
merged 4 commits into from
Jan 24, 2013
Merged

Store helper proxies instead of raw view contexts #438

merged 4 commits into from
Jan 24, 2013

Conversation

haines
Copy link
Contributor

@haines haines commented Jan 24, 2013

Any time we want to use Draper::ViewContext.current to access helper methods, we create a helper proxy to send the helpers because things like html_escape are private. Each decorator class gets its own stateless HelperProxy

This pull request changes this behaviour so that instead of having one proxy per decorator class, we create a proxy when we store the view context. All decorators then use this same helper proxy, which means that in tests we can do something like this

it "stubs a helper" do
  helper.stub content_tag: "hello"
  expect(decorator.h.content_tag).to eq "hello"
end

which is not currently possible because helper and decorator.h are different objects (#436).

I also made HelperProxy#method_missing define a method on the HelperProxy class so that future calls to the helper don't go through method_missing again, à la #89.

@haines haines mentioned this pull request Jan 24, 2013
@steveklabnik
Copy link
Member

@haines
Copy link
Contributor Author

haines commented Jan 24, 2013

Oops. No idea why this doesn't work on JRuby for Rails 3.x, but in any case looks like Rails 4 won't delegate non-public methods, so I'll change it to do an explicit send.

@haines
Copy link
Contributor Author

haines commented Jan 24, 2013

Sweet, it works now, after sorting out an incompatibility which I assume arose from an update to a JDBC driver (ugh!).

@carloslopes
Copy link
Contributor

Change tested and working here guys

steveklabnik added a commit that referenced this pull request Jan 24, 2013
Store helper proxies instead of raw view contexts
@steveklabnik steveklabnik merged commit 7a04619 into drapergem:master Jan 24, 2013
@haines haines deleted the issue_436 branch January 24, 2013 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants