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

Params are present in form builder on AJAX success #149

Closed
DannySantos opened this issue Oct 25, 2019 · 6 comments · Fixed by #150
Assignees
Milestone

Comments

@DannySantos
Copy link
Contributor

@DannySantos DannySantos commented Oct 25, 2019

In our Hanami project we've run into an issue when we use AJAX and the request is a success. If we want to re-render a section of the page without reloading or redirecting, any form that we attempt to re-render defaults to using the params for the values. At that point however, those params are outdated, and we don't want them to populate the form fields. This is particularly a problem when the params contain a collection, as the form builder is not well equipped to deal with them (it breaks when it attempts to use dig).

We've talked around it a bit and come up with a couple of potential solutions, though we're not 100% happy with either of them. Thought I'd open it as an issue and get some feedback, as one solution involves editing the form builder.

The first option is to have something like this in any view with a :js format:

def params
  return @_params if @_params
  
  @_params = locals[:params].dup
  @_params.to_h.keys.each{ |k| @_params.to_h.delete(k) }
  @_params
end

Using this method we get to retain the params object whilst emptying the part of it which the template actually uses.

The second method we played around with was to essentially add a params: false option to the form_for method. A very rough idea of how this might look would be something like adding this line to the method:

form_params = opts.fetch(:params, self.params)

Then passing form_params into the FormBuilder initializer:

FormBuilder.new(form, attributes, self, form_params, &blk)

Finally, in the FormBuilder initialize method, add form_params as and argument and change the following line:

@values      = Values.new(form.values, @context.params)

to:

@values      = Values.new(form.values, form_params)

Would love to hear some feedback on this. Thanks.

@jodosha

This comment has been minimized.

Copy link
Member

@jodosha jodosha commented Oct 25, 2019

@DannySantos I just want to double check: is your goal to clear all the form values after AJAX submission? Would this fix your problem? https://www.w3schools.com/jsref/met_form_reset.asp

@jodosha jodosha self-assigned this Oct 25, 2019
@DannySantos

This comment has been minimized.

Copy link
Contributor Author

@DannySantos DannySantos commented Oct 25, 2019

@jodosha No the goal isn't to clear all the values, it's to make sure that all of the values are correct, whether that means clearing them or, in the case of an update form, making sure they are populated by the attributes from the database and not the params. As our project has developed this has raised questions a few different times. Having just done a bit more hacking on this we've just realised that in the past we have had to get around this by specifying the values of individual fields. This also feels wrong, and like we're overwriting a piece of core Hanami functionality.

@jodosha

This comment has been minimized.

Copy link
Member

@jodosha jodosha commented Oct 25, 2019

@DannySantos Let me rephrase. We're talking about invalid data in form submission that is happening via AJAX.

  • In case of new form, you want to clear form inputs with invalid data, but keep the ones with valid data.

  • In case of edit form, you want to put back again data from the database in the form inputs with invalid ones, but leave as they are the form fields that weren't touched or fields with updated and valid data.

Is this correct?

@DannySantos

This comment has been minimized.

Copy link
Contributor Author

@DannySantos DannySantos commented Oct 28, 2019

@jodosha Sorry, I should have been more clear! We're not just talking about invalid data, but valid data too.

  • In the case of a new form or an edit form that fails validation I want to keep all of the fields that were submitted.

  • In the case of a new form that successfully creates the record I want to re-render a section of the page with AJAX which may contain the new form. In that case I want it to be rendered empty. Ideally the form should never know about the success params but currently it does, because we are still in the create action.

  • In the case of an edit form that successfully updates the record, again I want to re-render a section of the page containing the same edit form, or perhaps even a series of edit forms that all have the same name attributes. In this scenario it's important that the form(s) use the values from the entities rather than from the params. Two issues I have encountered as a result of this not being the case are:

  1. if you have multiple edit forms for the same resource on the same page (as a result of rendering one for each entity in a collection) then the params values will overwrite all of them. I got around this on our project by adding a value attribute to each of the individual fields, but this is obviously not an ideal solution.

  2. When re-rendering a form that contains a collection, the app breaks. We believe the reason behind this is the use of dig within the values.rb file. The point at which it breaks in form_builder is line 1632:

def _value(name)
  @values.get(
    *_input_name(name).split(/[\[\]]+/).map(&:to_sym)
  )
end

Which is called in the _attributes method:

def _attributes(type, name, attributes)
  attrs = { type: type, name: _displayed_input_name(name), id: _input_id(name), value: _value(name) }
  attrs.merge!(attributes)
  attrs[:value] = escape_html(attrs[:value])
  attrs
end

This could possibly be an issue that needs to be raised separately as none of the solutions we have come up with directly address this problem.

So back to the original point, to get around this the two ideas we've come up with involve either manually (but not very cleanly) clearing the params in a view method, or adapting the form_for method to allow you to specify that you do not want the params to be used for the form.

I hope that's a little clearer now.

@jodosha

This comment has been minimized.

Copy link
Member

@jodosha jodosha commented Oct 29, 2019

@DannySantos Thanks for the well crafted explanation. For the part regarding the params, I thought that it would be feasible to fix your problem by allowing to override params inline.

Would you please check if this patch fixes your params problem? #150

The PR contains an example to use the enhancement.


To try the PR, edit your Gemfile and add this line:

gem "hanami-helpers", git: "https://github.com/hanami/helpers.git", branch: "enhancement/form-for-inline-params"

Then run:

$ bundle update hanami-helpers
@jodosha jodosha added this to the v1.3.2 milestone Oct 29, 2019
@DannySantos

This comment has been minimized.

Copy link
Contributor Author

@DannySantos DannySantos commented Oct 31, 2019

@jodosha Worked perfectly! Thanks for the quick response on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.