eHealthAfrica / ehealth-bootstrap

Custom theme created for unifying Bootstrap across eHealth apps
http://ehealthafrica.github.io/ehealth-bootstrap/default/
0 stars 4 forks source link

Provide a visible `ng-invalid` #7

Open danse opened 9 years ago

danse commented 9 years ago

Angular automatically adds some classes which might help the user in understanding what is going on, and many eHealth applications are based upon Angular, so we have a good opportunity to improve the use experience in a coherent way by providing style for these classes. Along the way, we will probably have to figure out how to relate Angular-specific style with other parts of the themes which might be used with other technologies.

A good starting point is ng-invalid. This is added to Angular to every form control which has an invalid state, for example because it is required and empty. When this happens, the user is prevented from submitting, but without a styled ng-invalid she will not get any clue about what is going wrong, besides custom logic and style added by us case by case.

In Sense contact tracing, i added a quick block using the same style Bootstrap provides for .has-error:

input.ng-invalid, select.ng-invalid {
  border: 2px solid $brand-danger;
}
bnvk commented 9 years ago

@danse sounds good. I think you should create a separate file called angular.scss that gets compiled into it's own angular.css file that can be included in apps that need it. I think that makes most sense for now!

You might want to add a handful of these classes as boilerplate too, no?

danse commented 9 years ago

Yep a file dedicated to Angular seems the best solution. I do not feel confident in adding the other classes, this was a trivial case, just porting Bootstrap style to Angular classes, and i tested the result. I would not know how to style dirty, or pristine, and we would need to test them as well. The classes get added automatically so they are quite pervasive. Maybe an important case to add is the combination of untouched and invalid. In this case we do not want to show an error, because the user still has to type in. I would need to extend Bootstrap preview markup in order to display these combinations in a single page. Anyway these are subtleties, the most fundamental thing is give people a clue when they have an invalid form ... i think that we can adopt a bottom-up approach, adding style when we will need it

bnvk commented 9 years ago

@danse yah, definitely don't add all classes with styles that actually alter the UI, I was just thinking specing it out slightly more than the one class in there... so just empty CSS classes like:

input.ng-invalid, select.ng-invalid { }

The thinking being that it informs developers that they exist and could / should consider researching / using at a later date. But do what makes most sense to you!

tthew commented 9 years ago

Just to chime in here; I would suggest that any aliases that get created use SASS inheritance, variables, mixins and functions rather than hard coded values. Or rather than doing that just use ng-form validations and ng-class to apply the relevant bootstrap classes e.g:

 <div
   class="form-group"
   ng-class="{'has-error': showErrors && usersLoginForm.email.$error.required">
   <input
     type="email"
     class="form-control"
     name="email"
     ng-model="user.email"
     placeholder="e.g. example@unicef.org"
     required />
</div>

See http://getbootstrap.com/css/#forms-control-validation

danse commented 9 years ago

I looked into available variables and mixins, i should inherit from this class which is contained in a mixin, not sure how to do that. Sure the alternative could be to add that logic for every required control, but it is not very dry. Also, there is not always correspondence between Angular and Bootstrap classes.

For the moment i just wanted to open the topic and provide some minimal support ... somebody will be maintaining this repo anyway and he/she will decide how to conveniently factor the code ... maybe we will just have a single ng-invalid for the time being, without too much to factor.

Also picking input and select is quite lame, i could use :not(form) maybe? does it affect performances?

bnvk commented 9 years ago

Big :+1: to @tthew point of " any aliases that get created use SASS inheritance, variables, mixins and functions rather than hard coded values."

danse commented 9 years ago

So how would you structure this block?

danse commented 9 years ago

I ended up with something like

.ng-submitted {
  /* i want to exclude the forms here otherwise all their content will
  look wrong. i copied the style from Bootstrap's style for an input
  child of an element with `.has-error` */
  input.ng-invalid, select.ng-invalid {
    border: 2px solid $brand-danger;
  }
  /* a validation message is shown just if the corresponding
  validation error is set. we do not want to style this with danger
  before a first submit attempt */
  .validation-message {
    color: $brand-danger;
  }
}

In Sense Followup, in order to colour with danger the required fields and help texts, only after the user tried to submit the form once. I am going to copy this to Sense Dashboard. No time for putting this here now :unamused: