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

Grid Validation Implementation #11923

Merged
merged 172 commits into from
Sep 5, 2022
Merged

Grid Validation Implementation #11923

merged 172 commits into from
Sep 5, 2022

Conversation

MayaKirova
Copy link
Contributor

@MayaKirova MayaKirova commented Aug 2, 2022

Closes #6501

TODO:

  • Automation
  • Changelog

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@mddragnev
Copy link
Member

mddragnev commented Sep 2, 2022

On the tree grid add row sample, if you make the first two rows (with rowId 0 and 4) invalid. Afterwards if you call grid.validation.getInvalid() you will get an array with 2 invalid records. Then use the grid validation service clear method to clear only the validation errors of the first (grid.validation.clear(0)). Call again grid.validation.getInvalid() and observe that it returns empty array. This is because

 public clear(rowId?: any) {
        if (rowId) {
            this._validityStates.delete(rowId);
        } else {
            this._validityStates.clear();
        }
        this.updateStatus();
    }

the rowId is 0 and it is false. And the if block hits the else statement

@damyanpetev
Copy link
Member

On the dev grid validation sample, if you make the first row invalid by one of the cells and then use the public row api to get the errors it returns undefined. grid.getRowByIndex(0).errors is undefined, if you use the public cell api, the error is there. So the formGroup seems to not persist the errors that any of the form controls have.

@damyanpetev should we collect both the errors from the formGroup and the child formControl's for the public row API, or should the row be associated only with the formGroup?

I think we should stick to what normal form groups do: the errors are specific to where the validation is - if there was a validator on the form group itself (cross-field), then I'd expect the row to have errors. Otherwise, the errors are under the specific cell and you can't really expose them on the row without changing the error type (since there would need to be indication for which field the error object is, which is a map of maps, essentially what row.cell.errors is already :) )

MKirova added 2 commits September 2, 2022 16:15
…Type. Add interfaces for new grid events that have owner. Fix small issue in clear API.
@MayaKirova
Copy link
Contributor Author

@mddragnev Issues should be fixed now.

@mddragnev
Copy link
Member

mddragnev commented Sep 2, 2022

On the grid validation dev sample, when trying to add row make any of the cells invalid, the observe that grid.validation.getInvalid() does contain that row which is not added yet. If you cancel the adding, the validation service should not contain the invalid record for thar particular row, but it does.
NOTE: When canceling just editing it does work as expected

@mddragnev mddragnev added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Sep 2, 2022
mddragnev
mddragnev previously approved these changes Sep 2, 2022
<ng-template igxCellEditor let-cell="cell">
<input name="units" [(ngModel)]="cell.value" style="color: black" />
<ng-template igxCellValidationError let-cell='cell'>
<div *ngIf="cell.formGroup?.get(cell.column?.field).errors?.['forbiddenName']">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a bit of a left over, can be fixed later though

damyanpetev
damyanpetev previously approved these changes Sep 2, 2022
Copy link
Member

@damyanpetev damyanpetev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still a few bits to clean up around ARIA and dev demos, but overall LGTM

skrustev
skrustev previously approved these changes Sep 5, 2022
@dkamburov dkamburov dismissed stale reviews from skrustev and damyanpetev via aa93478 September 5, 2022 12:57
@skrustev skrustev self-requested a review September 5, 2022 13:03
@dkamburov dkamburov merged commit 073565d into master Sep 5, 2022
@dkamburov dkamburov deleted the mkirova/grid-reactive-forms branch September 5, 2022 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
grid: hierarchical-grid grid: tree-grid grid squash-merge Merge PR with "Squash and Merge" option status: localized ✅ status: verified Applies to PRs that have passed manual verification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validator service for igxGrid editing
9 participants