Hello
I'd like to bring into awareness a discussion that's going on in bookmarks PR
[1]. This PR demonstrates how to do client side validations using new
components for forms. E.g. this is how we define that the field is required
and can't be too long [2]. We have similar validations defined in our model.
One concern I had is that we'll duplicate the definition, both in UI and our
data model. I'd prefer to have UI validation rendered based on server
definition. There are cases where the UI field is not related to the specific
model attribute so we also need the option to explicitly define client
validations.
Similarly, strings that are normally rendered server side are now duplicated
at [3]. That can end up in out of sync error messages for users using UI and
API.
Of course this can be refactored later, but if we start to hardcode too many
things, the motivation for later refactoring will be lower. Also I'd like to
have good form helpers from the beginning since changes to them will have
impact on many plugins later.
I'm definitely not going to block the PR on this. But I'd like to hear
opinions of others, suggestions for improvements or any other feedback.
[1] https://github.com/theforeman/foreman/pull/4807
[2] https://github.com/theforeman/foreman/pull/4807/
files#diff-6011fb23e8f8bacb536ab442e57b874cR48
[3] https://github.com/theforeman/foreman/pull/4807/
files#diff-6254f272d0e11d7caaacec9ece0c6669R6
Thanks
···
--
Marek
Marek,
Thanks for bringing this up. I think you raised some good points and I do
agree, these are pain points. But I do prefer an iterative approach here,
where we don't get it perfect on the first spot. How can we be sure, what
we really need by not experimenting a bit? In my opinion, the react PRs are
already quite large. I personally prefer much smaller iterations. I think
we should define a common goal (maybe moving to a SPA all together, I don't
know) and try to reach that goal one step at a time.
I personally like the client-side validation, that's a lot of user value,
but I wouldn't have added it so early in the development process. It just
added more complexity.
To be honest, we define our data model already twice. Once in the rails
models and once in the API documentation. Maybe the way to go is to use
graphql to have a shared schema for both the client and the server? I don't
know the answer, but I believe, we need do something and learn from that.
It might not be perfect at first, but we'll eventually get there.
Timo
···
Am Dienstag, 19. Dezember 2017 17:59:28 UTC+1 schrieb Marek Hulán:
Hello
I'd like to bring into awareness a discussion that's going on in bookmarks
PR
[1]. This PR demonstrates how to do client side validations using new
components for forms. E.g. this is how we define that the field is
required
and can't be too long [2]. We have similar validations defined in our
model.
One concern I had is that we'll duplicate the definition, both in UI and
our
data model. I'd prefer to have UI validation rendered based on server
definition. There are cases where the UI field is not related to the
specific
model attribute so we also need the option to explicitly define client
validations.
Similarly, strings that are normally rendered server side are now
duplicated
at [3]. That can end up in out of sync error messages for users using UI
and
API.
Of course this can be refactored later, but if we start to hardcode too
many
things, the motivation for later refactoring will be lower. Also I'd like
to
have good form helpers from the beginning since changes to them will have
impact on many plugins later.
I'm definitely not going to block the PR on this. But I'd like to hear
opinions of others, suggestions for improvements or any other feedback.
[1] https://github.com/theforeman/foreman/pull/4807
[2] https://github.com/theforeman/foreman/pull/4807/
files#diff-6011fb23e8f8bacb536ab442e57b874cR48
<https://github.com/theforeman/foreman/pull/4807/files#diff-6011fb23e8f8bacb536ab442e57b874cR48>
[3] https://github.com/theforeman/foreman/pull/4807/
files#diff-6254f272d0e11d7caaacec9ece0c6669R6
<https://github.com/theforeman/foreman/pull/4807/files#diff-6254f272d0e11d7caaacec9ece0c6669R6>
Thanks
--
Marek
