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

Adding Haml support and teest #63

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

konung
Copy link
Contributor

@konung konung commented Oct 4, 2018

Hi Fran.
Unless I'm missing something HAML support ( Haml 5.0.4 with corresponding cells-haml) is available out of the box ( I haven't tried hamlit). Just added a test & gemspec - based off of SLIM.
You did all the hard work! 👍

@konung
Copy link
Contributor Author

konung commented Oct 4, 2018

Ignore pull request #62 - I did git add . from wrong directory level 🤕

@fran-worley
Copy link
Contributor

@konung Can you add an simple example in a branch in the demo repo too?

@fran-worley
Copy link
Contributor

Thanks so much for adding this - I never quite got around to adding 'formal' support for it!

@konung
Copy link
Contributor Author

konung commented Oct 4, 2018

Full stop. :(
So I was doing this with my eyes half closed last night - because I didn't see that haml version produces the same form twices. ( or rather some elements of the form are output twice before the form).
I've seen it before somewhere with simple_form I think, but I can't remember where or how I fixed.
Anyways this is related to #52

And will need to figure out a way around this first. I'll dive into formular later this week :)

@fran-worley
Copy link
Contributor

@konung we have the same issue with erb actually and why we had to add a blockless api see the erb tests https://github.com/trailblazer/formular/blob/master/test/fixtures/comment/erb/show/view/show.erb

@konung
Copy link
Contributor Author

konung commented Oct 4, 2018

Thank you Fran

This rabbit hole is insane. I figured out a temp solution and even the cause, but I'm not sure where to implement a permanent solution inside formular.

Problem
So this has to do with how HAML ( and ERB ) uses output buffer. These issues pointed me in the right direction ( just organizing it here for posterity)

Reference Reading

  1. http://trailblazer.to/gems/cells/cells4.html#html-escaping
  2. https://stackoverflow.com/questions/24985539/nested-capture-haml-helpers
  3. https://stackoverflow.com/questions/12437597/cannot-cache-from-decorator-draper

And these HAMLIT (not haml) issues

  1. Block-support and Capture k0kubun/hamlit#53
  2. https://github.com/hamlit/hamlit-block/issues/1
  3. Hamlit and cache blocks k0kubun/hamlit#56

Ad-Hoc Solution

So the solution that can be implement just in the HAML template is to capture contents of every block via capture_haml

%div New
= form(model, "/posts", path_prefix: :comment) do |f|
  - capture_haml do
    = f.input :id
    = f.textarea :body
    = f.checkbox :public, value: true
    = f.collection :replies do |reply|
      - capture_haml do
        = reply.input :content
    = f.nested :owner do |owner|
      - capture_haml do
        = owner.input :name
        = owner.input :email
    = f.input :uuid
    = f.submit value: "Submit"

I don't know enough about internals for formular to say if this can be fixed internally, but I think this might be the right spot?
https://github.com/trailblazer/formular/blob/master/lib/formular/html_block.rb

konung added a commit to konung/gemgem-sinatra that referenced this pull request Oct 4, 2018
@konung
Copy link
Contributor Author

konung commented Oct 4, 2018

@konung Can you add an simple example in a branch in the demo repo too?

I set it up (using the capture_haml approach above), but can't pull request to a non-existent branch on your repo - https://github.com/konung/gemgem-sinatra/tree/formular-haml-bootstrap4

Screenshot: https://user-images.githubusercontent.com/72493/46496670-417a4780-c7de-11e8-845c-5aaa9236be04.png

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.

2 participants