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

How to use form_for with ViewModel #260

Closed
st0mir opened this issue Jan 22, 2015 · 25 comments
Closed

How to use form_for with ViewModel #260

st0mir opened this issue Jan 22, 2015 · 25 comments

Comments

@st0mir
Copy link

st0mir commented Jan 22, 2015

How to use form_for helper in Cell::ViewModel?

Without including this helper I get error 'method undefined'. So I've tried to include it:
include ActionView::RecordIdentifier # this I need for method dom_class that is used by form_tag include ActionView::Helpers::FormHelper

but now input tags are not nested in form tag. It looks like this:
`<input ... />
<input ... />

`

Am I doing something wrong?

My rails version is 4.2.0

@apotonick
Copy link
Member

You have to include ActionView::Helpers::FormHelper into your cell, and then define #dom_class and #dom_id in your cell as an instance method. This is a dependency from simple_form or Haml, I don't know exactly why one needs this.

We can think about a cells helper module that provides that out-of-the-box but on the other hand I want to increase the pain so that users like you report to Rails core and probably push them to remove those dependencies from core. 😬

@st0mir
Copy link
Author

st0mir commented Jan 23, 2015

It still looks the same. I've added this two methods (delegate to ActionView::RecordIdentifier)

def dom_class(record, prefix = nil)
  ActionView::RecordIdentifier.dom_class(record, prefix)
end

def dom_id(record, prefix = nil)
  ActionView::RecordIdentifier.dom_id(record, prefix)
end

Both returns in my case 'new_article' (so I think value is ok), but form still doesn't render correctly:

<input type="text" name="article[title]" id="article_title" />
<form class="new_article" id="new_article"  ...></form>

I've also checked how it works in cells 4.0. So I included ActionView::Helpers::FormHelper to my cell class and implemented #dom_class and #dom_id methods (the same way as above). Here I get following exception:

NoMethodError - undefined method `encoding' for []:Array:
  actionview (4.2.0) lib/action_view/helpers/capture_helper.rb:197:in `with_output_buffer'
  haml (4.0.6) lib/haml/helpers/action_view_xss_mods.rb:5:in `with_output_buffer_with_haml_xss'
  actionview (4.2.0) lib/action_view/helpers/capture_helper.rb:38:in `capture'
  haml (4.0.6) lib/haml/helpers/action_view_mods.rb:52:in `capture_with_haml'
  actionview (4.2.0) lib/action_view/helpers/form_helper.rb:444:in `form_for'

Do you have any idea what I am doing wrong?

@seuros
Copy link
Member

seuros commented Jan 23, 2015

@st0mir can you try with my fork + addition of cells-haml ?
https://github.com/trailblazer/cells-haml

@st0mir
Copy link
Author

st0mir commented Jan 24, 2015

It works but not without problems.

I've got exception:

NameError - uninitialized constant Cell::Haml::Util:
  activesupport (4.2.0) lib/active_support/dependencies.rb:533:in `load_missing_constant'
  activesupport (4.2.0) lib/active_support/dependencies.rb:184:in `const_missing'
   () home/vagrant/.rvm/gems/ruby-2.1.5/bundler/gems/cells-haml-b99589c7e68d/lib/cell/haml.rb:13:in `set_output_buffer_with_haml'
   () home/vagrant/.rvm/gems/ruby-2.1.5/bundler/gems/cells-haml-b99589c7e68d/lib/cell/haml.rb:46:in `ensure in with_output_buffer'

so, I've changed lib/cell/haml.rb:13 to if ::Haml::Util.rails_xss_safe? && new_buffer.is_a?(ActiveSupport::SafeBuffer).

There was also problem with two missing methods on String:

undefined method `safe_concat'
undefined local variable or method `output_buffer'

so I had to include to String this modules:

include ActionView::Helpers::TextHelper
include ActionView::Context

@seuros
Copy link
Member

seuros commented Jan 24, 2015

Can you send a PR ?

@Titinux
Copy link

Titinux commented Jan 24, 2015

I am also trying to use form_for in a cell but I have a problem that is not mentioned here.

no implicit conversion of nil into String

I use rails 4.2, cells master and cells-haml form_tag_with_body branch from st0mir

I checked the form_tag_with_body method in lib/cell/haml.rb (cells-haml) and the content parameter is nil. If I force the content to be an empty string it does not crash.

I would be glad to tell you more details if needed.

@seuros
Copy link
Member

seuros commented Jan 24, 2015

You should use either my fork or cell-4 branch. Master is too hacky.
@apotonick we should merge cell cell-4 to master.

@Titinux
Copy link

Titinux commented Jan 24, 2015

Same error with the cells-4 branch.

But if I change the form_tag_with_body method like this

def form_tag_with_body(html_options, content)
  content ||= ""
  "#{form_tag_html(html_options)}" << content << "</form>"
end

my problem is solved. (no idea of other consequences 😓)

@seuros
Copy link
Member

seuros commented Jan 24, 2015

I'm going to fix it

@apotonick
Copy link
Member

This is just wrong per design - why is a view component library supposed to fix these kind of template engine details. I really hope we can set a usable standard with cells-haml etc. and push that knowledge back to Rails core itself for ActionView5.

I merged seuros cleanup, cells/cells-4 is now the authorative source for our future work. Thanks to @seuros for his work on extracting template engine specific logic to gems! ❤️ 🍻 ah no wait, he doesn't drink alcohol.... 🍵

@seuros
Copy link
Member

seuros commented Jan 24, 2015

🍸

@PikachuEXE
Copy link
Contributor

I need the gem cells-haml for Cell::Concept using cells 3.x.

@seuros
Copy link
Member

seuros commented Feb 27, 2015

cells-haml is for cells 4.x only.

@PikachuEXE
Copy link
Contributor

I guess I will just hack it myself Q_Q

@apotonick
Copy link
Member

Why do you need that for cells 3 anyway, @PikachuEXE?

@PikachuEXE
Copy link
Contributor

cells 3 also got Cell::Concept, and then I use simple_form in it...

@apotonick
Copy link
Member

But... I don't understand... how is the Concept cell in Cells 3 related to HAML bugs that are fixed in cells-haml? In Cells 3, these fixes are shipped with the cells gem itself?

@PikachuEXE
Copy link
Contributor

In one of my concept cell I need to use simple_form_for.
So I do this:

# Required by simple form for `form_for` I guess
include ActionView::RecordIdentifier
include ActionView::Helpers::FormHelper
include SimpleForm::ActionViewExtensions::FormHelper

Then it will out put escaped HTML
So I need to do include Cell::Haml too (code from cells-haml).
Also I need to include Cell::Haml last to make it work

@apotonick
Copy link
Member

Would using cells 4.0 be a choice for you?

@PikachuEXE
Copy link
Contributor

For now I would be using my hacks.
I got many "old cells" which inherits from Cell::Rails
I guess I don't have time refactoring until April

@apotonick
Copy link
Member

Ok, if your hacks work, then I'm happy. Sorry for the inconvenience - everything will be better in Cells 4.

@shkm
Copy link

shkm commented Aug 11, 2015

Just wanted to point out that this still doesn't work in Cells 4 (I'm not sure if it should; I'd like it to, but that's not my decision). Thankfully, @PikachuEXE descended like an angel, and sprinkled magical dust upon my code, allowing it to just work.

I'm using this concern to get it working with Formtastic:

module FormsForCells
  extend ActiveSupport::Concern

  included do
    include ActionView::RecordIdentifier
    include ActionView::Helpers::FormHelper
    include ActionView::Helpers::TextHelper
    include ActionView::Context
    include Formtastic::Helpers::FormHelper

    include Cell::Haml
  end
end

@apotonick
Copy link
Member

You have to tell the formtastic guys to fix that in their code. They are not including necessary helpers in their modules and assume that everything is automatically/magically there, which is the case in Rails but not in Cells or in other Ruby environments.

Do me a favour and open a ticket there, telling them to include the necessary modules in Formtastic::Helper (or whatever the name of the "main" helper is). I could add a fix to Cells, but this way, it will never be fixed correctly. 🍻

@PikachuEXE
Copy link
Contributor

Don't put and "workaround" (not fix) in this gem, no.
Another negative "rails style" 💣

@contentfree
Copy link

Thanks for the idea, @shkm. Using Cells 4 (and cells-rails and cells-haml and simple_form). I had to make a concern / module to include in my cells as the list of includes was as long as yours.

It would be nice if cells-rails or cells-haml would do some of this themselves, but I guess there's both pros and cons to having stuff automagically included into Cell::Concept

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

No branches or pull requests

7 participants