Skip to content

Commit

Permalink
Render grid hidden inputs in a table cell. Closes #339.
Browse files Browse the repository at this point in the history
One of the effects of the changes made for the upgrade to Formtastic 2.1
(#227) apparently had the effect of wrapping the hidden inputs for question
ID and response ID in `li` tags. Those inputs were being rendered directly
into a `tr` (i.e., outside of any cell). This was illegal nesting, so the
browser relocated the `li`s (and the inputs) to somewhere they could legally
be rendered.

This seems to have moved them enough that they were no longer matched by the
jQuery selector used for submitting partial results. They were still in the
`form` overall and so were submitted when the whole page was submitted.

Moving the inputs to somewhere an `li` can legally be nested fixes this
problem.
  • Loading branch information
rsutphin committed Jun 21, 2012
1 parent 5b5b71b commit 03cbc8a
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions app/views/partials/_question_group.html.haml
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@
%th   %th  
- ten_questions.each_with_index do |q, i| - ten_questions.each_with_index do |q, i|
%tr{:id => "q_#{q.id}", :class => "q_#{renderer} #{q.css_class(@response_set)}"} %tr{:id => "q_#{q.id}", :class => "q_#{renderer} #{q.css_class(@response_set)}"}
- if q.pick == "one" %th
- r = response_for(@response_set, q, nil, g) = q.split_text(:pre)
- i = response_idx # increment the response index since the answer partial skips for q.pick == one - if q.pick == "one"
= f.semantic_fields_for i, r do |ff| - r = response_for(@response_set, q, nil, g)
= ff.input :question_id, :as => :quiet - i = response_idx # increment the response index since the answer partial skips for q.pick == one
= ff.input :id, :as => :quiet unless r.new_record? = f.semantic_fields_for i, r do |ff|
%th= q.split_text(:pre) = ff.input :question_id, :as => :quiet
= ff.input :id, :as => :quiet unless r.new_record?
- q.answers.each do |a| - q.answers.each do |a|
%td= render a.custom_renderer || '/partials/answer', :g => g, :q => q, :a => a, :f => f %td= render a.custom_renderer || '/partials/answer', :g => g, :q => q, :a => a, :f => f
%th= q.split_text(:post) %th= q.split_text(:post)
Expand Down

4 comments on commit 03cbc8a

@jefflunt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this commit might misalign grids, shifting the answer labels on the top of the grid one table cell to the left, so that the first answer label is over the questions instead of the first answer.

This is because the %th that's added in each row of the questions/radio buttons needs to be matched with a %th in the top row, so that every row of the table has the same number of cells. If the answers and answer text are misaligned, it can become unclear which answers/radio buttons actually go with which answer text, which results in invalid data.

@Sivli-Embir
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rsutphin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@normalocity can you provide a sample survey that demonstrates the problem you're describing? This commit should not change the number or position of cells in the header row — it just moves some illegally nested (non-cell) elements to a legal position.

@CMToups I don't think your issue is related to this commit. (This commit is altering a view only; the issue you're describing would be in the model or the parser.) Unfortunately, I'm not familiar with how the :correct option is intended to work. Would you please as your question on the mailing list? Someone should be able to help you there —it may indeed be that you've found a bug, but the mailing list is the best place to ask.

@jefflunt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I'm going to have to withdraw my comment. I was testing against a different version of surveyor with a customized view, and didn't realize it. After testing it against the current version of master it seems to work as expected.

Please sign in to comment.