chore: add `required` option to LiveSelect
Is there any obstacle that LiveSelect currently doesn't support a required attr? If not, then I can work on adding that :)
I actually came here to ask for the same thing. The omission of this basic HTML functionality is somewhat bewildering.
@itamm15 required is an attribute for front-end HTML validations controlled by the browser. LiveView forms work with validations that run on the backend. That's the reason why I never thought of adding it.
Can you please elaborate a bit on when/how you would use it and why do you believe it's necessary?
If we decided to add it, an immediate problem that I can see is that in tags mode the text input element will be empty even if something is selected, and the front-end validation will fail, even it really shouldn't.
Hi @maxmarcon, thanks! The reason is simple - what we have to do now if we want to require the select to not be empty is;
- Assign a variable on
mountlevel,is_form_valid: false, - You the variable in the
button,<button disabled={!is_form_valid} ...>, - Then, add the validation via
phx-chang="validate-form", - Check on the server level, whether the select is not empty - if is, then don't change the
is_form_validstate, elseis_form_valid = true. - The button is enabled, and the user can proceed with some action.
This is our current workaround for the required option. This whole workflow could be simply substituted by the required option.
Regarding tags - sorry, I am not that aware of this functionality, but maybe we could simply drop the required option for this mode? (in other words, forbid required option to be assigned to the LiveSelect with tags mode)
Hi and thanks. Ok so you're not using Ecto schemas and changesets to validate your form? Because if so I believe all you'd have to do would be to use validate_required/3 and bind the disabled attribute of your button to the valid status of the form.
Regarding the changsets for form - generally, yes, we do use. But this one is a bit more complicated, I would say, because we use interceptors (designed by our RAP system), and before actually executing the function triggered from the click on the form, the interceptor is being fired (and the required field is needed for the interceptor logic, not the changeset itself).
Scenario without required: We have a create event for a resource and also defined interceptors to check the permissions before executing the create event.
- User goes to the form and doesn't fill the select (that's meant to be required, so we do normally require/validate it as described in my first comment),
- The user clicks the
createbutton, and interceptors are executed. Hence, boom 💥 happens 'cause the input hasn't been filled!
Hope that makes sense 😄
I don't know what you mean by interceptor or by RAP. But if I understood you correctly, you have some front-end third-party code that expects a required attribute in the input field? It's the only case where I can see that adding the attribute might be justified, because in all other cases you can and should do everything on the server
Apologies for not being clear!
- RAP - roles and permissions system
- interceptors - https://hexdocs.pm/phoenix_live_view/Phoenix.LiveView.html#attach_hook/4
Sample LiveView (pseudocode)
def LiveView do
use LiveView, :live_view
on_mount(LiveView.Interceptors, {:create, :can_create})
def render(assigngs) do
~H"""
<form phx-change="create">
<.live_select id="some_attr_that_should_be_required" />
</form>
"""
end
def mount(_params, _session, socket), do: {:ok, socket}
def handle_info(:create, params, socket) do
# creation logic
end
end
def LiveView.Interceptors do
def mount(interceptors, params, socket) do
value_from_live_select = params["some_attr_that_should_be_required"]
attach_hook(interceptors, ..., fn -> end)
end
end
As the value from LiveSelect is used in the interceptors (because it is required attr in the interceptors to check permissions), we are not able to validate it in the create/2 event. I hope that makes it clearer!
ok thanks, it's getting a bit clearer, still a lot of questions. I'll pick 2:
- why using hooks to verify if a field of a form is set instead of doing it in the
submitorchangeevents? - which stage of the LiveView lifecycle are you tapping into and how are the forms parameters being accessed by the hook? (Your code doesn't show that)
- So, we are checking if the field was settled in the form - from this perspective, we require it to be settled. It is just needed for further checks (e.g. db queries). (and as stated previously, now we mimicked the behaviour of the
requiredoption forLiveSelectwith thechangeevent, but therequiredoption would just be simpler and definitely sufficient for us, as we just require this field to be settled, not actually validated), - So, this is done automatically by/within the
attach_hook/4! 😄
https://www.youtube.com/watch?v=8UQLPQDqkMQ
sorry, but you didn't really answer my questions. I know how attach_hook works, that's why I asked you which stage you're attaching to (handle_params, handle_event etc. ?) and how the code in your hook is accessing the parameters of the form.
Regardless of the way in which you access the form, I don't understand why you need required attribute at the HTML level. You have the form with the parameters I presume? Then you can just check whether the field you need is set 🤷 . Please tell me what I'm missing and maybe send me some code with a complete example of what you're trying to do. Thanks.
Sorry for all of the confusion, @maxmarcon!
So, my main argument for the required option would be just the ease of usage. As you correctly pointed, we can achieve the same with the phx-change and then validation on the server level. But that's just a few more lines of code and an additional call to the server in exchange for 1 line and a client check.
The required attribute is helpful when we have a form with a Submit button. If the user clicks the Submit button without entering anything in the input, the browser will automatically prompt the user to fill in the field. It's as simple as that.
Correct. It's a best practice when it comes to HTML forms to rely on built-in HTML validations as much as possible. The required attribute in particular is really nice because not only does it let you enforce the validation with zero code, but also saves you bandwidth.
@olafbado see my comment above. You can do all this in LiveView
@egeersoz I don't think you're saving any bandwidth. Remember, every time the user changes something in the form in LiveView, this is a round-trip to the server. You can always bind everything in your HTML, including whether a submit button is disabled, to the state of the changeset that is backing your form. This is the basic idea behind LiveView forms, and how errors are usually rendered.
Anyway, you all seem to be really eager to have this, and I just saw that Phoenix core components have required in the list of global attributes allowed in the input fields.
Maybe we can create a MR that lets the global attributes through (including required) and sets them in the text input field.
Would this be ok?
Hi @maxmarcon! What you mean is that all of the attrs provided by Phoenix will be supported by LiveSelect, right?
Remember, every time the user changes something in the form in LiveView, this is a round-trip to the server.
I don't believe this is accurate, and I verified it by looking at the traffic in my browser's console. Server roundtrips on user changes happen only if the form has a phx-change handler, which is optional. The most common scenario is to simply use a phx-submit, where all changes are local and the data is sent (and validated) only when the form is submitted.
Some changes necessitate "real-time" validation using phx-change. Many others don't, and in those scenarios forcing round-trips is a waste of bandwidth and server processing power.
I don't believe this is accurate, and I verified it by looking at the traffic in my browser's console. Server roundtrips on user changes happen only if the form has a phx-change handler, which is optional. The most common scenario is to simply use a phx-submit, where all changes are local and the data is sent (and validated) only when the form is submitted.
Yes, I was assuming one is using phx-change and phx-submit because this is the basic idea behind LiveView: all validations and all the logic reside on the server. I highly doubt that the most common scenario is just using phx-submit as you write. Just take a look at the first paragraph of the "Form Bindings" section in the LiveView docs:
To handle form changes and submissions, use the phx-change and phx-submit events. In general, it is preferred to handle input changes at the form level, where all form fields are passed to the LiveView's callback given any single input change
Pretty unambiguous statement I would say.
To me, mixing front-end and back-end validations and logic in LV (unless strictly necessary) is mising the point of the framework. Moreover, if you're worried about this level of bandwidth saving (passing small events over a websocket) then I think you shouldn't use LV but a front-end framework. Anyway, just my opinions and I'm not gonna tell you how you have to design your application 😉
@itamm15
Hi @maxmarcon! What you mean is that all of the attrs provided by Phoenix will be supported by LiveSelect, right?
The idea would be to add an attribute to the form component along these lines (code taken from the Phoenix core components, just an example):
attr :rest, :global,
include: ~w(accept autocomplete capture cols disabled form list max maxlength min minlength
multiple pattern placeholder readonly required rows size step)
where you add a catchall rest attribute for the global HTML attributes, and you render it like this in the text input:
<input .... {@rest} />
It's important we take special care that @rest doesn't allow overriding other attributes set internally by the component, most importantly the class attribute. If you want to take a stab at it, you're very welcome.
Hey @maxmarcon! Okay, that's what I thought initially! Yup, I would like to take care of that. Are you sure we need all of these attrs, though? 🤔 (if you want to, we can discuss this matter in the MR)
@itamm15 No, my gut feeling is that we should start small and only allow the attributes we need (required in this case) but I don't know how you can forbid the HTML class attribute in the :global component attribute type (haven't looked at this stuff in a long time)
I guess considering the different options is part of the job of working on the MR. Yes, further discussion is best handled there. Thanks