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

multiple scenarios how views can get out of sync when trying to validate and fix user inputs #446

Closed
fopsdev opened this issue Jul 4, 2016 · 7 comments
Labels

Comments

@fopsdev
Copy link

fopsdev commented Jul 4, 2016

I'm just showing how easy the view can be brought out of sync using existing out of the box concepts.
Please have in mind that i really like to expose my models directly to the ui and i don't like to work on clones or use other development patterns. Thats why i add some validation to keep them clean.

valueConverters:
https://gist.run/?id=0ae5f6f0ff127e404c9076dce866902d

changeHandler:
https://gist.run/?id=f2fae5b7695b76a3c820caaa4bced084

Similar Issues and their solutions :
All the following issues are related to simple validation scenarios.
The solutions are technically brilliant and appreciated but i think there should be an easier way to accomplish this

#353
Solution: Additional usage of a Signaler
#442
Solution: Additional usage of a Signaler
http://stackoverflow.com/questions/37882659/force-view-to-be-updated-from-within-a-valueconverter/37890664#comment63703880_37890664
Solution: Additional Signaler, BindingBehaviour, CustomElement
Btw. this posts mention some other use cases as well

So long story short:
After changing a value which goes to the model (be it from the valueConverter or in the changedHandler) the view should be updated as well

@jdanyow
Copy link
Contributor

jdanyow commented Jul 5, 2016

The signaler isn't intended to be a fix for binding problems. I disagree with the recommendations in the issues/posts that you linked. The signaler's purpose is to signal binding updates for events that aren't "property change" or "collection mutated" related (eg the current time changed or the current locale changed).

Although it's tempting to try and make a numeric input control using only a value converter, in practice, it's not enough. A custom element is the preferred way to create input controls. With a custom element you get to decide when to update the bindable "value" property and with what value. Value converters don't have the same degree of flexibility and have no way to add "behavior".

#442 mentions changes being "reverted" when they actually never happen due to the setter not accepting the value. This means no change notification or binding update happens as a result.

@fopsdev
Copy link
Author

fopsdev commented Jul 5, 2016

Thanks Jeremy for taking your time.
I totally agree that the signaler should not be used to solve binding problems.Thats one of the reasons i've come up with this issue :)

As you also recognised it's very tempting to use the valueConverter to convert values when they are written back to the model ;)

In 90% of the cases when i use the fromView() Method i ran into the sync issue mentioned here.
(Don't think ColorPicker here, think more like Text Inputs converted to Ints, Dates, ....)

So your answer implies that i need to create different custom elements just for the sake of a "clean" dataentry? In my case this would mean that i should create a custom element for:

  • date entry (I don't use datepickers, just quick date text entry which resolves to a date on some clever assumptions)
  • numeric int entry
  • numeric float entry
    (i see those two could be merged into one when using some attributes maybe...)

I don't have a problem with that because i'll avoid valueConverter-fromView() from now on and i'm able to create those custom-elements. But i'm a bit scared that fresh users will also run into this and generate Issues.
So to keep somehow the "easyness" of Aurelia for those simple usecases we would need either:
After changing a value which goes to the model (be it from the valueConverter or in the changedHandler) the view should be updated as well
or
A Set of custom elements which handle those inputs

I clearly prefer the View Update because the work already done with fromView() and propertyChanged() would be much,much more useful then.

So i would like to keep this issue open.

Btw. i recognised that the value converter gist didn't show off the issue nicely. i've changed it:
https://gist.run/?id=0ae5f6f0ff127e404c9076dce866902d

@fopsdev
Copy link
Author

fopsdev commented Jul 6, 2016

@jdanyow @EisenbergEffect could you please reopen this issue?
i would like to have more opinions on that.

@fopsdev
Copy link
Author

fopsdev commented Jul 6, 2016

just another "view not updated" issue:
aurelia/framework#341
a different issue but shows that valueConverters really need a little bit more love, so why not slap 2 flies with one clap :) :
https://www.bountysource.com/issues/32109394-value-converters-should-support-triggering-updates

@jdanyow
Copy link
Contributor

jdanyow commented Jul 6, 2016

@fopsdev No need to duplicate #442 or #353. Please continue the relevant parts of this discussion in those issues.

Here's a simple number-input element and number-value attribute. I hope these will help illustrate why a custom elements/attributes are a better fit for accepting numeric/date/etc input.

https://gist.run/?id=d9d8dd9df7be2dd2f59077bad3bfb399

@jdanyow jdanyow reopened this Jul 6, 2016
@fopsdev
Copy link
Author

fopsdev commented Jul 6, 2016

i understand. thanks
my goal is to have something like a summary for view sync issues in here.
thanks for the inspiring gist. they really help me.
but my main concern stays. new users will be misleaded by the term "value converter". they will run down that path (meaning use valueconverters for validation purposes) like i did and hit the wall :)

@EisenbergEffect
Copy link
Contributor

Closing as a duplicate of other issues listed above.

We'll try to clarify some explanations in our documentation so that ValueConverter to get used for the wrong purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants