woylie / flop_phoenix

Components for pagination, sortable tables and filter forms using Phoenix, Ecto and Flop
MIT License
385 stars 43 forks source link

remove dependency on phoenix_html_helpers #299

Open woylie opened 9 months ago

woylie commented 9 months ago

298 updated phoenix_html to 4.0 and updated the calls to functions that were moved to phoenix_html_helpers. In the long run, the dependency on those functions should be removed completely.

linusdm commented 9 months ago

In the documentation there is still mentioning of Phoenix.HTML.Tag (for setting the no_results_content option when rendering tables) and Phoenix.HTML.Form.inputs_for (when customising the filter form).

The first one might be replaced with a regular heex render, like this:

defmodule FlopHelpers do
  use MyAppWeb, :html

  def table_opts do
    assigns = %{}

    [
      no_results_content: ~H"""
      <p>no results found</p>
      """
    ]
  end
end

For the second use, I don't have a better alternative than using PhoenixHTMLHelpers.Form.inputs_for right now. But could probably be improved with regular components too.

woylie commented 9 months ago

Replacing inputs_for with the component version is not possible, since the Phoenix.HTML.FormData implementation for Flop.Meta relies on the fields option being passed, but Phoenix.Component.inputs_for/1 doesn't allow to pass additional options at the moment. I'm not sure how to go about it.

woylie commented 9 months ago

It would probably make sense to add an attr :opts, :list, default: [] to Phoenix.Component.inputs_for/1. The documentation for Phoenix.HTML.FormData.to_form/2 states:

The options have their meaning defined by the underlying implementation but all shared options below are expected to be implemented. All remaining options must be stored in the returned struct.

Since Phoenix.Components.inputs_for/1 uses the to_form function, it should be able to pass any options that the protocol implementation requires.

linusdm commented 9 months ago

That's a great insight. Thanks for taking the initiative upstream! That will allow a cleaner interface, more idiomatic in line with the new Phoenix Components. Very nice!

jelkand commented 7 months ago

Hello, I've been playing around with the use of Phoenix.Components.inputs_for/1 in order to style some of my own filter fields using live components, and I get a lot of console errors in the browser about duplicated IDs (screenshot below). My best guess is that something about the offset option is not being respected when provided via Phoenix.Components.inputs_for/1. It seems to work fine with Phoenix.HTMLHelpers.Form.inputs_for/4. Unfortunately I'm not able to spot any differences in implementation that would cause that.

image
woylie commented 7 months ago

Yes, some flop_phoenix tests start failing when I switch to the inputs_for component because the IDs aren't as expected. I haven't found the issue yet.

jelkand commented 6 months ago

I have been looking into this further and it looks like the issue stems with these changes: https://github.com/phoenixframework/phoenix_live_view/commit/1e169b8c29ad108c5be61a69d0008e14c99bfa1c

This adds the concept of a _persistent_id to inputs_forand in doing so, it reassigns the id the Flop form set using the offset.

I believe the way to address this is to set the _persistent_id param in the HTML Form so that the .inputs_for component can pick it up and use it to generate proper IDs. That said I'm not too experienced with LiveView and I'm not sure if that is the way folks intended the _persistent_id to be used.

I'm happy to put up a PR to demonstrate, even if it ends up not being the right solution. Hopefully this can help your investigation as well.

woylie commented 6 months ago

@jelkand That is very helpful, thanks for looking into this! I didn't have a closer look yet, but I'd like to understand better what the purpose of the _persistent_id, and whether this is something that could or should be solved on the LiveView side.

jelkand commented 6 months ago

I agree, and adding a _persistent_id to the params of the flop_phoenix form makes for a far-reaching change, especially considering that params is used internally to handle filters/query params etc. I believe the _persistent_id is used to support dynamically adding and removing form elements or forms while maintaining their ordering, but I haven't used it in anger.