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

Core: input validation highlight isn't working #14036

Closed
brandyscarney opened this issue Feb 16, 2018 · 7 comments · Fixed by #15856
Closed

Core: input validation highlight isn't working #14036

brandyscarney opened this issue Feb 16, 2018 · 7 comments · Fixed by #15856
Assignees

Comments

@brandyscarney
Copy link
Member

Ionic version: (check one with "x")
(For Ionic 1.x issues, please use https://github.com/ionic-team/ionic-v1)
[ ] 2.x
[ ] 3.x
[x] 4.x

I'm submitting a ... (check one with "x")
[x] bug report
[ ] feature request

screen shot 2018-02-15 at 4 35 35 pm

I'm not sure of how we want to do this with core/angular. I'd prefer if we didn't have Angular specific styles in core, so either we add the styles to our angular package or we add our own Ionic classes for validation based on the Angular controls (my preference).

Here are the current styles:


// Input highlight mixin for focus, valid, and invalid states
@mixin ios-input-highlight($highlight-color) {
  border-bottom-color: $highlight-color;
}

// Show the focus highlight when the input has focus
@if ($input-ios-show-focus-highlight) {
  // In order to get a 2px border we need to add an inset
  // box-shadow 1px (this is to avoid the div resizing)

  .item-ios.item-input.item-input-has-focus .item-inner {
    @include ios-input-highlight($input-ios-highlight-color);
  }

  // The last item in a list has a border on the item, not the
  // inner item, so add it to the item itself
  .list-ios .item-input.item-input-has-focus:last-child {
    @include ios-input-highlight($input-ios-highlight-color);

    .item-inner {
      box-shadow: none;
    }
  }
}

// Show the valid highlight when it has the .ng-valid class and a value
@if ($input-ios-show-valid-highlight) {
  .item-ios.item-input.ng-valid.item-input-has-value:not(.item-input-has-focus) .item-inner {
    @include ios-input-highlight($input-ios-highlight-color-valid);
  }

  .list-ios .item-input.ng-valid.item-input-has-value:not(.item-input-has-focus):last-child {
    @include ios-input-highlight($input-ios-highlight-color-valid);

    .item-inner {
      box-shadow: none;
    }
  }
}

// Show the invalid highlight when it has the invalid class and has been touched
@if ($input-ios-show-invalid-highlight) {
  .item-ios.item-input.ng-invalid.ng-touched:not(.item-input-has-focus) .item-inner {
    @include ios-input-highlight($input-ios-highlight-color-invalid);
  }

  .list-ios .item-input.ng-invalid.ng-touched:not(.item-input-has-focus):last-child {
    @include ios-input-highlight($input-ios-highlight-color-invalid);

    .item-inner {
      box-shadow: none;
    }
  }
}

Also, we might want to have the angular classes passed down to the native input.

screen_shot_2018-02-16_at_12_49_07_pm

And here's how we did it in v3:


function setControlCss(element: Ion, control: NgControl) {
  if (!control) {
    return;
  }
  element.setElementClass('ng-untouched', control.untouched);
  element.setElementClass('ng-touched', control.touched);
  element.setElementClass('ng-pristine', control.pristine);
  element.setElementClass('ng-dirty', control.dirty);
  element.setElementClass('ng-valid', control.valid);
  element.setElementClass('ng-invalid', !control.valid);
}
@kensodemann
Copy link
Member

Modified the control value accessors to set the following classes conditionally based on the associated ng-* classes:

  • ion-invalid
  • ion-valid
  • ion-touched
  • ion-untouched
  • ion-dirty
  • ion-pristine

@brandyscarney brandyscarney modified the milestones: @ionic/core 0.0.3, @ionic/core 0.0.4 Mar 7, 2018
@brandyscarney
Copy link
Member Author

@jonmikelm
Copy link

Any news on this issue? We are developing an app that makes heavy use of angular reactive forms. We wonder if ionic form components will support by default input validation highlighting for angulr forms or we will have to implement our own styling for "ng-*" classes.

We are working with beta.7 and have verified that validation is working correctly and "ng-" and "ion-" classes are being applied to the ion-input element. But the red outline doesn't show up in invalid inputs.

brandyscarney added a commit that referenced this issue Oct 4, 2018
Adds the following CSS properties to item:

```
--highlight-color-focused
--highlight-color-valid
--highlight-color-invalid
--highlight-height
```

This also fixes an issue where we were showing the highlight on items
with no lines, and shows inset vs full properly.

fixes #14036 fixes #9639 fixes #14952 closes #15690
brandyscarney added a commit that referenced this issue Oct 5, 2018
Adds the following CSS properties to item:

```
--highlight-color-focused
--highlight-color-valid
--highlight-color-invalid
--highlight-height
```

This also fixes an issue where we were showing the highlight on items
with no lines, and shows inset vs full properly. Adds documentation and tests for input focus.

fixes #14036 fixes #9639 fixes #14952 closes #15690
@joel-daros
Copy link

joel-daros commented Oct 23, 2018

@brandyscarney This issue needs to be reopened.

The border color doesn't change when the form is invalid.

** TESTED in Beta-13 **

  1. Create a reactive form with a validator like maxLength:
    this.profileForm = this.formBuilder.group({
      name: ['', [Validators.required, Validators.maxLength(3)]],
    });
  1. Create a form and add the reactive text field:
<ion-content padding>
  <form [formGroup]="profileForm">

    <ion-item>
      <ion-label>Name:</ion-label>
      <ion-input formControlName="name"></ion-input>
    </ion-item>

    <pre>{{profileForm.value | json}}</pre>
    <pre>{{profileForm.status}}</pre>

  </form>
</ion-content>

When the field form get invalid the line color below the field didn't change.

@brandyscarney
Copy link
Member Author

@MrSparklle Could you create a new issue for this? I would like to keep this issue specific to adding it to the @ionic/core package (the bare CSS needed to add highlighting) and then have a separate issue for any issues with the Angular integration (the class being added to the wrong place). Thank you! 🙂

@joel-daros
Copy link

No problem Brandy. Issue opened: #16052

@ionitron-bot
Copy link

ionitron-bot bot commented Nov 22, 2018

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Nov 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants