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

Add row basic functionality #8093

Merged
merged 25 commits into from
Sep 25, 2020
Merged

Add row basic functionality #8093

merged 25 commits into from
Sep 25, 2020

Conversation

mddragnev
Copy link
Member

@mddragnev mddragnev commented Sep 4, 2020

Related to #5946 and IgniteUI/trial-watermark#50

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

@ChronosSF ChronosSF marked this pull request as ready for review September 9, 2020 15:48
@ChronosSF ChronosSF changed the title Add row initial functionality Add row basic functionality Sep 9, 2020
@mddragnev mddragnev requested a review from ChronosSF September 10, 2020 15:23
recordRef: row,
addRow: true
};
collection.splice(parentRecordIndex + 1, 0, rec);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this pipe runs after all other pipes, there may be cases when the row should not be shown below the parent record index. For example, if you have a grid with master-detail views, if the new row is displayed under the parent (master row) it would visually break the hierarchy since the new row will appear above the details view:
image

In those cases it may be better to show the add row above it.
Also the add row should not allow expand/collapse operations

projects/igniteui-angular/src/lib/grids/common/pipes.ts Outdated Show resolved Hide resolved
projects/igniteui-angular/src/lib/grids/common/pipes.ts Outdated Show resolved Hide resolved
ChronosSF
ChronosSF previously approved these changes Sep 14, 2020
return;
}
if (commit) {
this.gridAPI.submit_add_value();
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to generate an update transaction. There's no need to generate an update transaction before adding the row.

MayaKirova
MayaKirova previously approved these changes Sep 18, 2020
@3phase 3phase self-assigned this Sep 21, 2020
@ChronosSF ChronosSF added 💥 status: in-test PRs currently being tested and removed 🛠️ status: in-development Issues and PRs with active development on them labels Sep 23, 2020
@ChronosSF ChronosSF requested a review from simeonoff September 23, 2020 11:54
@3phase
Copy link
Contributor

3phase commented Sep 23, 2020

When adding the Add Row functionality to the MRL template, you cannot edit the newly added rows in the row that appears beneath the click target of the action strip.

bug_add_new_row_mrl

@3phase
Copy link
Contributor

3phase commented Sep 23, 2020

When adding a new row into a grid with enabled filtering, the grid is not added (the onRowAdded event won't be fired at all).

bug_add_new_row_filtering

%igx-grid__addrow-snackbar {
position: absolute;
z-index: 5;
left: 35%;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This property should be the interpolated version of the $left variable for RTL compatibility purposes.

Suggested change
left: 35%;
#{$left}: 35%;

Also, are you sure you want to use percentages as the unit to set the position of the snackbar? If yes, how did you come up with those numbers?

@ChronosSF ChronosSF added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Sep 25, 2020
@ChronosSF ChronosSF merged commit 203bc7a into master Sep 25, 2020
@ChronosSF ChronosSF deleted the mdragnev/add-row-feat branch September 25, 2020 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
grid: general grid: row-adding version: 10.2.x ✅ status: verified Applies to PRs that have passed manual verification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants