Skip to content

Grid Master Detail #6119

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 65 commits into from
Nov 27, 2019
Merged

Grid Master Detail #6119

merged 65 commits into from
Nov 27, 2019

Conversation

MayaKirova
Copy link
Contributor

@MayaKirova MayaKirova commented Nov 5, 2019 •
edited by ChronosSF
Loading

Closes #5880

To do:

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
  • This PR includes API docs for newly added methods/properties
  • 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
  • This PR includes behavioral changes and the feature specification has been updated with them

@MayaKirova MayaKirova requested a review from ChronosSF November 5, 2019 08:27
@MayaKirova MayaKirova mentioned this pull request Nov 5, 2019
Merged
@MayaKirova MayaKirova added the ❌ status: awaiting-test PRs awaiting manual verification label Nov 8, 2019
@MayaKirova MayaKirova requested a review from kdinev November 8, 2019 09:22
@MayaKirova MayaKirova marked this pull request as ready for review November 8, 2019 09:22
ChronosSF
ChronosSF previously approved these changes Nov 11, 2019
View reviewed changes
rkaraivanov
rkaraivanov reviewed Nov 21, 2019
View reviewed changes
projects/igniteui-angular/src/lib/grids/cell.component.ts
Comment on lines +866 to +872
// persist focused cell
const el = this.grid.selectionService.activeElement;
if (isVirtualized && el) {
const cell = this.grid.gridAPI.get_cell_by_visible_index(el.row, el.column);
if (cell) {
cell.nativeElement.focus();
}
Copy link
Member

Choose a reason for hiding this comment

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

This won't always persist the focus on the right cell element as notifyChanges is run on the next detection cycle and the layout of the grid body can shift up/down

Copy link
Member

Choose a reason for hiding this comment

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

Related to #6191

@ChronosSF ChronosSF self-requested a review November 21, 2019 13:27
@ChronosSF ChronosSF added 🛠️ status: in-development Issues and PRs with active development on them and removed 💥 status: in-test PRs currently being tested labels Nov 21, 2019
@MayaKirova MayaKirova added ❌ status: awaiting-test PRs awaiting manual verification and removed 🛠️ status: in-development Issues and PRs with active development on them labels Nov 22, 2019
@ChronosSF ChronosSF added 💥 status: in-test PRs currently being tested and removed ❌ status: awaiting-test PRs awaiting manual verification labels Nov 25, 2019
@nrobakova
Copy link
Contributor

nrobakova commented Nov 27, 2019 •
edited
Loading

Hi,
I tested this PR and found some issue related to it:

  • getSelectedData() returns null values when master detail is in included in the selection

  • When rowSelection="multiple" and navigate with Tab from master detail. The row selector checkbox is focused, but the first cell in the next row should be focused.

  • When filter and then collapse master detail an error is returned. Steps to reproduce:

    1. Open dev sample gridMasterDetail
    2. Expand first row.
    3. Filter column Contact name with value 'th'
    4. Expand filtered row
    5. Clear filter
    6. Collapse first row
      An error in console is returned

  • When navigate with tab from rowEditng last cell , the master detail is selected instead of the row editing Cancel button

@ChronosSF ChronosSF added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Nov 27, 2019
@kdinev kdinev merged commit d6c950b into master Nov 27, 2019
@ChronosSF ChronosSF deleted the mkirova/grid-master-detail branch December 3, 2019 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChronosSF ChronosSF ChronosSF approved these changes

@rkaraivanov rkaraivanov rkaraivanov approved these changes

@dkamburov dkamburov Awaiting requested review from dkamburov

@simeonoff simeonoff Awaiting requested review from simeonoff

@kdinev kdinev Awaiting requested review from kdinev

Assignees

@dkamburov dkamburov

@ChronosSF ChronosSF

@skrustev skrustev

@mddragnev mddragnev

Labels
grid: general version: 9.0.x ✅ status: verified Applies to PRs that have passed manual verification
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow master-detail style visualization with igxGrid
8 participants
@MayaKirova @nrobakova @ChronosSF @rkaraivanov @dkamburov @kdinev @skrustev @mddragnev