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

Show number of defined columns as rowChangesCount for new row with batch editing #12234

Merged
merged 15 commits into from
Nov 14, 2022

Conversation

MonikaKirkova
Copy link
Contributor

@MonikaKirkova MonikaKirkova commented Oct 26, 2022

Closes #12212

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

@dkamburov
Copy link
Contributor

@MonikaKirkova please update the spec and check the last item from the checklist.

@dkamburov dkamburov requested a review from kdinev November 9, 2022 08:06
@@ -3219,6 +3219,9 @@ export abstract class IgxGridBaseDirective extends DisplayDensityBase implements
Object.keys(obj).forEach(key => isObject(obj[key]) ? changes += f(obj[key]) : changes++);
return changes;
};
if (this.transactions.getState(this.crudService.row.id)?.type === TransactionType.ADD) {
return this._columns.filter(c => c.field).length;
Copy link
Member

Choose a reason for hiding this comment

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

Are there columns without a field and why are they excluded from the count?

Copy link
Contributor Author

@MonikaKirkova MonikaKirkova Nov 9, 2022

Choose a reason for hiding this comment

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

The field property of the unbound columns is undefined and there was a meeting where we discussed to include only the defined columns which are included in the data.

Copy link
Member

@kdinev kdinev Nov 9, 2022

Choose a reason for hiding this comment

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

@MonikaKirkova OK, makes sense. However, I'm not finding anything in the docs about unbound column. I didn't know we support this, nor can I find that it's done without specifying a field property value. @ChronosSF This last part of the comment should be addressed. It's not directed at Monika specifically :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By unbound column I mean a column, which field property is not set and is not part of the data (not explicitly marked as unbound). For example the column with delete button in this demo.

Copy link
Member

Choose a reason for hiding this comment

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

@MonikaKirkova I know what it is :) I was making the comment for @ChronosSF, because us, as developers of the product - we know how to create something like an unbound column, but developers who consume the library usually search for this in the docs!

Copy link
Member

Choose a reason for hiding this comment

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

Logged IgniteUI/igniteui-docfx#3388 to address this

@ddaribo ddaribo added 💥 status: in-test PRs currently being tested ✅ status: verified Applies to PRs that have passed manual verification and removed ❌ status: awaiting-test PRs awaiting manual verification 💥 status: in-test PRs currently being tested labels Nov 11, 2022
@teodosiah teodosiah requested a review from kdinev November 11, 2022 13:51
kdinev
kdinev previously approved these changes Nov 11, 2022
@MonikaKirkova MonikaKirkova requested a review from kdinev November 11, 2022 15:42
@teodosiah teodosiah merged commit 6bc92f6 into master Nov 14, 2022
@teodosiah teodosiah deleted the mkirkova/feat-12212 branch November 14, 2022 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
grid: row-adding version: 15.0.x ✅ status: verified Applies to PRs that have passed manual verification
Projects
None yet
6 participants