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

Validator service for igxGrid editing #6501

Closed
kdinev opened this issue Jan 21, 2020 · 24 comments · Fixed by #11923
Closed

Validator service for igxGrid editing #6501

kdinev opened this issue Jan 21, 2020 · 24 comments · Fixed by #11923
Assignees
Labels
grid: cell-editing grid: general grid: row-editing size: L 🧰 feature-request ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged.

Comments

@kdinev
Copy link
Member

kdinev commented Jan 21, 2020

Is your feature request related to a problem? Please describe.

As a developer, I would like to be able to reuse some or all of my forms validation logic with the igx-grid editing.

Describe the solution you'd like

Currently, if I want to validate the igx-grid row or cell editing and display different validation messages, stop user navigation, etc. I have to write an entirely custom validation logic. There's also no unified way to plug validation into the grid editing. I would like some mechanism, like the grid transaction service, which allows me to define validation logic and events it triggers on, similar to forms validation.

Describe alternatives you've considered

Write an entirely custom validation logic.

Additional context

This affects maintainability of applications using igniteui-angular.
Consider #4135

@github-actions
Copy link

There has been no recent activity and this issue has been marked inactive.

@github-actions github-actions bot added the status: inactive Used to stale issues and pull requests label May 10, 2020
@radomirchev radomirchev added 🧰 feature-request and removed 🧰 feature-request status: inactive Used to stale issues and pull requests labels Jun 9, 2020
@radomirchev radomirchev reopened this Jun 9, 2020
@github-actions
Copy link

There has been no recent activity and this issue has been marked inactive.

@github-actions github-actions bot added the status: inactive Used to stale issues and pull requests label Aug 15, 2020
@kdinev kdinev removed the status: inactive Used to stale issues and pull requests label Aug 17, 2020
@github-actions
Copy link

There has been no recent activity and this issue has been marked inactive.

@github-actions github-actions bot added the status: inactive Used to stale issues and pull requests label Oct 26, 2020
@radomirchev radomirchev removed the status: inactive Used to stale issues and pull requests label Oct 26, 2020
@github-actions
Copy link

There has been no recent activity and this issue has been marked inactive.

@github-actions github-actions bot added the status: inactive Used to stale issues and pull requests label Dec 26, 2020
@github-actions github-actions bot closed this as completed Jan 2, 2021
@radomirchev radomirchev removed the status: inactive Used to stale issues and pull requests label Feb 25, 2021
@radomirchev radomirchev reopened this Feb 25, 2021
@github-actions
Copy link

There has been no recent activity and this issue has been marked inactive.

@github-actions github-actions bot added the status: inactive Used to stale issues and pull requests label Apr 27, 2021
@github-actions github-actions bot closed this as completed May 5, 2021
@radomirchev radomirchev reopened this May 10, 2021
@kdinev kdinev removed the status: inactive Used to stale issues and pull requests label May 11, 2021
@github-actions github-actions bot added the status: inactive Used to stale issues and pull requests label Mar 6, 2022
@Lipata Lipata removed the status: inactive Used to stale issues and pull requests label Mar 6, 2022
@mikerentmeister
Copy link

Any idea when this tentatively might be released?

@kdinev
Copy link
Member Author

kdinev commented Apr 11, 2022

@mikerentmeister This item is in our backlog plans for the current quarter. We will update the item with an estimated release date soon! @radomirchev

@github-actions
Copy link

There has been no recent activity and this issue has been marked inactive.

@github-actions github-actions bot added the status: inactive Used to stale issues and pull requests label Jun 11, 2022
@radomirchev radomirchev assigned dkamburov and ChronosSF and unassigned Lipata and zdrawku Jun 16, 2022
@kdinev kdinev removed the status: inactive Used to stale issues and pull requests label Jun 20, 2022
@mikerentmeister
Copy link

Any updates on this in terms of estimated delivery?

@kdinev
Copy link
Member Author

kdinev commented Jul 1, 2022

@mikerentmeister We kicked this item off this week. What we're planning is to expose FormControl instances for the grid row editors, which will allow the same validation logic, which is passed into other forms in an application, to also be accepted by the grid columns. We also discussed updating the read-only rendering to reflect validation state (invalid column values) after cell editing has been performed on them and the editor is no longer visible. We're currently in the state of updating the editing specification to reflect these. @MayaKirova and @damyanpetev will update the issue further after the specification update is in place, so there can be a discussion on it.

@mikerentmeister
Copy link

Some special things that I would love to be considered:

  • The ability to pass a FormGroup object for row validations as a whole (We already have ones generated by a metadata service).
    • The FormGroup would have to be cloned for each row, or a way to designate a function to be called to create a FormGroup. I.E. formGroupCreator: () => UntypedFormGroup
    • Having a FormGroup instance exposed provides a way out of the box to do more complex validations like ColumnA is required when ColumnB is true.
  • It probably will, but just wanted to call out that this needs to work with server paged/virtualized grids as well, when the rows load on demand.
  • When rowEditable is true, a way to prevent the user from leaving the row when there are errors in the row.
  • A way to add a red border around a row when a cell is invalid, Potentially an error icon added as a suffix as well.
    • Please make sure this can be included (perhaps configured) through igxCell and igxCellEditor templates as well.
  • In batchEdit mode, a service/function to get all current cells with errors in the grid.
    • Also a way to prevent committing transactions until all errors are fixed.
  • The ability to show an error message for a cell, above/below it. Perhaps only showing when the user is in edit mode of a particular cell. That way error messages are not overlapping with other cells that might have errors.
    • Special consideration here is that FormGroups/Controls allow you to return errors, but they don't currently have a great way that I know of to specify an error message for a given error. Perhaps an EventEmitter can be exposed when the error message is being prepared to be shown, so that users can set/modify the error message. In the object passed through the event emitted I would love to have access to the cell and the FormGroup for the row.
    • Please make sure this can be included (perhaps configured) through igxCell and igxCellEditor templates as well.
  • The ability to prevent users from leaving a cell when there are errors. (there are probably already ways of doing this through the cellEdit/cellEditExit events -- just wanted to call it out)
  • An EventEmitter, perhaps "statusChanged", that would emit events when a grid is changed from valid to invalid, and vice versa
  • Async validations might be something to consider as well, but honestly is not required for my use cases.

@MayaKirova
Copy link
Contributor

@sdimchevski @dkamburov Since there are some suggestions in the above comment: #6501 (comment) related to UI, specifically how the invalid state will be reflected in the editor/cell/row it might also be good to kick off some design for those. Maybe some design for how validation errors should appear in editors and also some invalid cell state design.

@MayaKirova
Copy link
Contributor

@mikerentmeister
Wanted to ask a few additional questions regarding some of the points:

  • A way to add a red border around a row when a cell is invalid, Potentially an error icon added as a suffix as well.
    Please make sure this can be included (perhaps configured) through igxCell and igxCellEditor templates as well.

    Do you mean a template for the whole row or just the cell and/or the editor?
    Also does this assume editing is already complete and an invalid value was added as a pending transaction or (in the case where no transaction service was used) submitted in the data?

  • An EventEmitter, perhaps "statusChanged", that would emit events when a grid is changed from valid to invalid, and vice versa

    What would constitute an "invalid" state for the grid?

    • The moment an invalid value is entered in an editor
    • The moment an invalid value is added as a pending traction (if transaction service is used)
    • The moment an invalid value is added to the grid data (pending invalid transaction is commited or no transaction service was used so changes directly reflect in the data).

To me both of these make sense only in the context of a transaction service, where you can track the changes and their validity. So they should only apply if you have transaction service and any validity state (of the grid, row or cells) when you're not currently in edit mode should be based on the pending transactions.

Still wanted to ask to make sure that would align with your expectations.

@mikerentmeister
Copy link

mikerentmeister commented Jul 7, 2022

Do you mean a template for the whole row or just the cell and/or the editor?

I misspoke, I meant a red border around an invalid cell. Probably a class on the igx-grid-cell like "igx-grid__td--invalid". I just want to make sure it works whether or not you use a custom igxCell/igxCellEditing template. When using batch editing, the border should persist, even if you leave the cell. Only when the user is editing an invalid cell, this is how I would envision the error message being shown:

image

Also does this assume editing is already complete and an invalid value was added as a pending transaction or (in the case where no transaction service was used) submitted in the data?

Probably both if possible, but I would settle for just the first if the second is too hard

What would constitute an "invalid" state for the grid?

In my opinion, probably the moment an invalid value is entered in an editor. The reason being is that's generally when Angular's FormControl would be marked as invalid

To me both of these make sense only in the context of a transaction service, where you can track the changes and their validity. So they should only apply if you have transaction service and any validity state (of the grid, row or cells) when you're not currently in edit mode should be based on the pending transactions.

When batch edit mode is off I would still see value in having a red border around a cell you're currently editing if it's invalid

@ChronosSF ChronosSF added the 🛠️ status: in-development Issues and PRs with active development on them label Jul 11, 2022
@dkamburov
Copy link
Contributor

@sdimchevski We'll need a design for those validation error messages. Can you take a look on those when you have time. Can be added directly into the spec - https://github.com/IgniteUI/igniteui-angular/wiki/Grid-Validation-Specification

@MayaKirova
Copy link
Contributor

@dkamburov @damyanpetev @kdinev I've updated the specification with the initial draft of the dev experience and API so that we can start reviewing and discussing it.
There's also a POC branch with a validation sample that follows the suggested spec (UI is not final and will probably change once we have a final UX design) : https://github.com/IgniteUI/igniteui-angular/tree/mkirova/grid-reactive-forms

@sdimchevski
Copy link

Spec is updated with design hand-off link in Figma

@mikerentmeister
Copy link

The spec looks like it will meet all of my needs, thanks very much!

@MayaKirova
Copy link
Contributor

@dkamburov @ChronosSF @damyanpetev @kdinev Specification is updated and ready for review/discussion.
POC is up-to-date and available at: #11923

@radomirchev radomirchev added ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged. and removed 🛠️ status: in-development Issues and PRs with active development on them labels Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
grid: cell-editing grid: general grid: row-editing size: L 🧰 feature-request ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.