search

varvet / godmin

Admin framework for Rails 5+
http://godmin-sandbox.herokuapp.com
MIT License
486 stars 47 forks source link

[WIP] Remove instance variables #243

Closed jensljungblad closed 6 years ago

jensljungblad commented 6 years ago

This PR attempts to solve #220.

Started looking at this and it will be tricky, but I like the concept. First I replaced all instance variables with helper methods, which I got working. Unfortunately our code is a bit hard to follow. The service object is modified during the request cycle by different parts of the code, so things need to happen in a particular order for things to work, which is why we set the instance variables in the order that we do. Not nice, we should try and fix that as well.

Then I tried replacing the helper methods with locals, which is what we want. That way, variables aren't automatically available to all views and partials, but have to be explicitly passed, which is nice. However, we have a problem with some of our view helpers, such as the translation and form helpers. They require access to resource_class and resource_parents. We could pass them to the translation helper, but it's trickier with the form helper which mimics the Rails helper.

Do we want to expose some things as helper methods, available everywhere and some things passed down as locals?

jensljungblad commented 6 years ago

We could leave instance variables in the controller layer, but send locals to the view layer, if that makes it easier.

For the translation helper, pass it the needed locals.

For the form helper, perhaps we can use Rails built in method of passing resource parents..?