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

Deprecate rowID and rowData in interfaces #10617

Closed
6 tasks done
hanastasov opened this issue Nov 23, 2021 · 15 comments · Fixed by #13427
Closed
6 tasks done

Deprecate rowID and rowData in interfaces #10617

hanastasov opened this issue Nov 23, 2021 · 15 comments · Fixed by #13427

Comments

@hanastasov
Copy link
Contributor

hanastasov commented Nov 23, 2021

Following the removal of rowID and rowData properties from IgxRowDirective and all public classes implementing the RowType interface, considering the following:

Deprecate the old rowID and rowData from any or all of the following:

and introduce the new key or primaryKey (already done with #12394) and data

More stuff to be addressed in this PR:

  1. For better readibility and maintenance, destructure IGridEditDoneEventArgs to ICellEditDoneEventArgs
    and IRowEditDoneEventArgs as follows:

IGridEditDoneEventArgs to:

  • ICellEditDoneEventArgs
  • IRowEditDoneEventArgs

IGridEditEventArgs to

  • ICellEditEventArgs
  • IRowEditEventArgs

Grid,interface.ts shoud look like:

cellEditEnter: EventEmitter<ICellEditEventArgs>;
cellEdit: EventEmitter<ICellEditEventArgs>;
cellEditDone: EventEmitter<ICellEditDoneEventArgs>;
cellEditExit: EventEmitter<ICellEditDoneEventArgs>;
rowEditEnter: EventEmitter<IRowEditEventArgs>;
rowEdit: EventEmitter<IRowEditEventArgs>;
rowEditDone: EventEmitter<IRowEditDoneEventArgs>;
rowEditExit: EventEmitter<IRowEditDoneEventArgs>;

DONE b92d8ff?show-viewed-files=true&file-filters%5B%5D=

  1. For rowAdd and rowDelete emit new interface, which is cancellable - maybe IRowDataEventArgs is the proper candidate.
    2.1 - see comment on line 555 in grid,interface about this change - in the end we can reconsider if this will be introduced with 16.1 or other more major version
    2.2 - add rowDelete to GridType, build correct object to be emitted

  2. Change rowAdd and rowDelete types for treeGrid and Pivot to be compatible with GridType

  3. The primaryKey argument also crept into add and edit events, not only rowDeleted:

  1. The rest of the events, which emitted rowID until now - see if needed to migrate rowID to rowKey or key. For row events, it makes sense to be key, while for cell events it makes sense to be rowKey. Still, if cell event to emit rowKey in difference to row events, which will be emitting key, is a big effort, keep cell events to also emit key and make sure it is properly documented, so that a user knows what is reading through eventArgs.key

  2. Make a list of the breaking changes and think of way to avoid them. Damyan can help here too.

@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 Jan 23, 2022
@hanastasov hanastasov removed the status: inactive Used to stale issues and pull requests label Jan 25, 2022
@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 Mar 27, 2022
@hanastasov hanastasov removed the status: inactive Used to stale issues and pull requests label Mar 28, 2022
@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 28, 2022
@github-actions github-actions bot closed this as completed Jun 5, 2022
@hanastasov hanastasov removed the status: inactive Used to stale issues and pull requests label Jun 6, 2022
@hanastasov hanastasov reopened this Jul 18, 2022
@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 Sep 17, 2022
@hanastasov hanastasov removed the status: inactive Used to stale issues and pull requests label Sep 21, 2022
@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 Nov 21, 2022
@hanastasov hanastasov removed the status: inactive Used to stale issues and pull requests label Nov 21, 2022
@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 Jan 21, 2023
@hanastasov hanastasov removed the status: inactive Used to stale issues and pull requests label Jan 24, 2023
@wnvko
Copy link
Contributor

wnvko commented Apr 12, 2023

I would go with key everywhere. I know we just added primaryKey, but let change it to key.

@hanastasov
Copy link
Contributor Author

I would go with key everywhere. I know we just added primaryKey, but let change it to key.

Just to remind tt went from key to primaryKey in #12394 by demand.

@damyanpetev
Copy link
Member

@wnvko The argument was explicitly renamed from key to primaryKey because it was not the same type - as in it'll never switch to the row object to use as identifier when a primaryKey on the Grid is not defined and will be empty instead. So it's indeed primary key only and best not to mix it with the key concept on row.

IIRC, the argument was actually only on the delete event btw (since that's where it was needed), yet it might've crept up to some other events? Did we end up with events that have both key and primaryKey? Cuz that'd be super awkward lol

@hanastasov
Copy link
Contributor Author

Thanks.

  1. No, we didn't end up with events that have both key and primaryKey
  2. Yes, the primaryKey argument also crept into add and edit events, not only rowDeleted. That's totally my mistake as I did not extend the row deleted arguments, as task was logged, but for me it "made sense" to do the same for edit and add events.

Will try to fix this in current PR.

@hanastasov hanastasov added 🛠️ status: in-development Issues and PRs with active development on them and removed ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged. labels Apr 12, 2023
@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 25, 2023
@hanastasov hanastasov removed the status: inactive Used to stale issues and pull requests label Jun 26, 2023
@github-actions
Copy link

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

@hanastasov
Copy link
Contributor Author

Plan for version 17.

Copy link

github-actions bot commented Jan 9, 2024

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 Jan 9, 2024
@hanastasov hanastasov removed the status: inactive Used to stale issues and pull requests label Jan 10, 2024
hanastasov added a commit that referenced this issue Feb 5, 2024
* refactor(Grid): deprecate rowID and rowData props from the grid eventArgs #10617

* chore(*): address review comment for editDoneArgs

* chore(*): update the tests according new API changes

* chore(*): fix tests accordinly newly addressed API changes

* chore(*): fix rowEditDone event args

* fix(*): do not clear summaries when row edit is not complete #10617

* chore(*): fix linting errors

* chore(grid): separate grid and cell event interfaces, events reorganization

* chore(*): add proper description for interfaces

* chore(*): fix the description of the deprecated properties

* feat(*): expose new event arguments for grid rowAdd and rowDelete  #10617

* chore(*): add key and deprecate primaryKey event args

* chore(*): address the issues in the test due to changes in the event arguments

* chore(*): fix some linting errors

* chore(*): fix failing test

* chore(*): fix failing tests

* chore(*): change the version for the deprecation of rowId and rowData props

* chore(*): describe in the changelog the deprecated props rowID and  rowData

* chore(*): remove some leftover comment

* chore(*): apply some of the review comments

* Update CHANGELOG.md

Co-authored-by: Konstantin Dinev <kdinev@mail.bw.edu>

* chore(*): address some more comments

* feat(*): reintroduce primaryKey prop and introduce rowKey instead key #10617

* test(Grid): fix the test with reintroduced props primaryKey and rowKey #10617

* feat(*): remove the deprecation of rowData prop and remove data prop #10617

* chore(*): drop the new interfaces entirely

* test(Grid): address the latest changes in the gridEdit events #10617

* chore(*): remove cellID and and column from IRowDataCancelableEventArgs

* chore(*): deprecate data and introduce rowData in IBaseRowDataEventArgs

* chore(*): deprecate key, introduce rowKey

* chore(*): remove unnecessary added key prop in IRowDataEventArgs

* chore(*): unify rowKey prop for the rest of the interfaces

* chore(*): update the changelog

* chore(*): remove unused interface

* chore(*): update changelog for pivot

* chore(*): update changelog for pivot

* Apply suggestions

minor formatting and redundant comments

* chore(*): add wrongfully removed comments

* chore(*): should not use deprecated props internally

* chore(*): fix indentation

* feat(*): IRowDataCancelableEventArgs to extend IGridEditEventArgs #10617

* chore(*): update the deprecation version and the changelog description

* chore(*): changelog enhcaements

* chore(*): fix typo in interace name

* Update projects/igniteui-angular/src/lib/grids/summaries/grid-summary.service.ts

* Update projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid.component.ts

* Update projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid-crud.spec.ts

* Update projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid-crud.spec.ts

* Update projects/igniteui-angular/src/lib/action-strip/grid-actions/grid-editing-actions.component.spec.ts

* chore(*): fix incorrect work usage

* chore(grid): cleanup redundant comments

* chore(grid): row data cancelable spelling; consistent w/ base interface

* docs(grid): update events changelog

* docs(changelog): grid event arg deprecations separate section

---------

Co-authored-by: Hristo Anastasov <HAnastasov@infragistics.com>
Co-authored-by: Stamen Stoychev <sstoychev@infragistics.com>
Co-authored-by: Konstantin Dinev <kdinev@mail.bw.edu>
Co-authored-by: Damyan Petev <damyanpetev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment