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

Improve test: Add '[]' suffix to multi-select name attr #90

Closed

Conversation

ftes
Copy link
Contributor

@ftes ftes commented Jun 17, 2024

Extracted from #74

Align PageView with IndexLive:
<select multiple> should use name attr ending in [].

Even though the tests do not currently fail (decode multiple values), an actual Phoenix app would fail (decode only a single value).

As a follow-up, it might be good to make the Static driver stricter to emulate real-world behaviour.

https://github.com/elixir-plug/plug/blob/v1.16.0/lib/plug/parsers/urlencoded.ex
https://github.com/elixir-plug/plug/blob/v1.16.0/lib/plug/conn/query.ex#L24

  Lists are created with `[]`:

      iex> decode("foo[]=bar&foo[]=baz")["foo"]
      ["bar", "baz"]

@ftes
Copy link
Contributor Author

ftes commented Jun 18, 2024

Somewhat related to #77, #79

germsvel pushed a commit that referenced this pull request Jul 5, 2024
Fixes #77
Replaces #79, #90, #93

## Goal
Ensure same form handling semantics as Phoenix controllers and LiveView.

## Details
Track form data as a flat list of `{name, value}` tuples instead of nested maps. 

This can be read straight from the DOM and needs no further conversions.

Example:
```ex
form_data = [
  {"user[name]", "Ada"},
  {"user[subscribe][]", "email"},
  {"user[subscribe][]", "push"}
]
```

Defer transforming that flat list into a nested data structure: When submitting form or evaluating other LiveLive `phx-*` callbacks.

Use [Plug.Conn.Query.decode/4](https://github.com/elixir-plug/plug/blob/v1.16.0/lib/plug/conn/query.ex#L9) to transform flat list into nested data structure. Reason: Phoenix Controllers and LiveView do the same (see #77 (comment)).
@germsvel
Copy link
Owner

germsvel commented Jul 5, 2024

@ftes I'm going to close this since I think it's fixed by #94 (right?). If it's not, happy to reopen it.

@germsvel germsvel closed this Jul 5, 2024
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