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

[Time Conductor] Don't update model while typing #324

Closed
wants to merge 6 commits into from
Closed

Conversation

VWoeltjen
Copy link
Contributor

Instead of updating the underlying model for values from a date-time fields during typing, do so only when user input is complete (on a blur and/or submit.) If text is invalid when user input is complete, restore it to a valid state.

This prevents constraints in the time conductor from being enforced prematurely (while the user is typing), which can result in undesired behavior as shown in #259.

Note that this does not include any visual indication of constraint violation (e.g. start is greater than end time), as described in this comment of the original issue. Will file a follow up issue for these cases; submitting to merge in its current state due to impending code freeze. (Changes are sufficient to make #259 irreproducible.)

Validate while a user types, but only update the underlying
value in the model on a blur event. #259
...to reflect changes to when validation and model updates
occur.
When pressing enter (e.g. submitting) text input into
data-time fields, update the underlying model (as done
on tab out.)
...to reflect that a date-time field is only invalid
while the user is still entering bad input (will be
reset to a valid state on blur etc.)
...when tabbing out and/or submitting bad input.
@VWoeltjen
Copy link
Contributor Author

Author Checklist

  1. Changes address original issue? Y *
  2. Unit tests included and/or updated with changes? Y
  3. Command line build passes? Y
  4. Expect to pass code review? Y

* Original issue is not reproducible after these changes but only a subset of the designed solution is implemented; filed #325 as a follow-up issue to address this.

@VWoeltjen VWoeltjen added this to the Banks milestone Nov 20, 2015
@larkin
Copy link
Contributor

larkin commented Nov 20, 2015

This is some nonstandard stuff-- we should implement forms according to the HTML spec and use common form handling standards. Otherwise, we will have inferior experience on different devices and be unusable to anyone with assistive devices, and our code will get quite convoluted; user agents provide considerable assistance in handling forms-- they will handle the things that would normally trigger form submit, and often will provide better form navigation tools.

Basically:

  • mct-control should not be it's own form.
  • one or more mct-controls should be wrapped in forms by whatever is using them.
  • validation based logic can trigger on changes in field values or on form-submit.
  • any other logic should be triggered on form submit (usually form submit has two possible outcomes: "form is valid and do something" or "form is invalid and show errors").

(This is yet another reason why angular two-way binding is a mess; it often subverts proper form handling behavior)

Here's basic form entry on iPhone; note the functionality it picks up from being a form:

image1

So, to provide time-conductor specific feedback:

  • the time conductor should wrap all the input elements it wishes to submit into a form.
  • the time conductor should only update from the form values when a form submit occurs.
  • it should trigger form submits as a result of specific input events, e.g. on blur of an input field, it can automatically submit the form. I'd argue that tabbing from "start date" to "end date" should not cause the form to submit-- instead, it should wait for neither date field to be focused before triggering a form submit. (Note that the user agent will trigger a form submit if the user presses "enter" in an input field)

The edit properties dialogue also needs the same treatment, as it has similar issues. I would save so much time if I could press "enter" after typing a folder name to save it. Since that is out of scope for this issue, we can file a follow-up for that to bring forms back in line with HTML standards.

Sending back to @VWoeltjen.

@larkin larkin assigned VWoeltjen and unassigned larkin Nov 20, 2015
@VWoeltjen
Copy link
Contributor Author

Closing this PR; essentially a rewrite of these changes is called for above, makes sense to start from a new branch.

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