Skip to content

Commit

Permalink
Merge pull request #7269 from IgniteUI/rkaraivanov/keyboard-fixes
Browse files Browse the repository at this point in the history
fix(for-of): Revert change to small scrolls
  • Loading branch information
zdrawku committed May 12, 2020
2 parents 90ebb4a + 255ce3c commit 720f0e8
Show file tree
Hide file tree
Showing 13 changed files with 143 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1216,21 +1216,19 @@ describe('igxCombo', () => {
fixture.detectChanges();
let items = fixture.debugElement.queryAll(By.css(`.${CSS_CLASS_DROPDOWNLISTITEM}`));
let lastItem = items[items.length - 1].componentInstance;
let lastItemIndex = lastItem.index;
expect(lastItem).toBeDefined();
lastItem.clicked(mockClick);
await wait(30);
fixture.detectChanges();
expect(dropdown.focusedItem.index).toEqual(lastItemIndex);
expect(dropdown.focusedItem).toEqual(lastItem);
dropdown.navigateItem(-1);
await wait(30);
fixture.detectChanges();
expect(virtualMockDOWN).toHaveBeenCalledTimes(0);
lastItemIndex = lastItem.index;
lastItem.clicked(mockClick);
await wait(30);
fixture.detectChanges();
expect(dropdown.focusedItem.index).toEqual(lastItemIndex);
expect(dropdown.focusedItem).toEqual(lastItem);
dropdown.navigateNext();
await wait(30);
fixture.detectChanges();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
TemplateRef,
TrackByFunction,
ViewContainerRef,
ViewRef,
AfterViewInit
} from '@angular/core';

Expand Down Expand Up @@ -850,27 +849,89 @@ export class IgxForOfDirective<T> implements OnInit, OnChanges, DoCheck, OnDestr
this.sizesCache,
0
);

if (newStart + this.state.chunkSize > count) {
newStart = count - this.state.chunkSize;
}

const prevStart = this.state.startIndex;
const diff = newStart - this.state.startIndex;
this.state.startIndex = newStart;

if (diff) {
this.onChunkPreload.emit(this.state);
if (!this.isRemote) {
this.fixedApplyScroll();

// recalculate and apply page size.
if (diff && Math.abs(diff) <= this.MAX_PERF_SCROLL_DIFF) {
diff > 0 ? this.moveApplyScrollNext(prevStart) : this.moveApplyScrollPrev(prevStart);
} else {
this.fixedApplyScroll();
}
}
}

return inScrollTop - this.sizesCache[this.state.startIndex];
}

/**
* @hidden
* The function applies an optimized state change for scrolling down/right employing context change with view rearrangement
*/
protected moveApplyScrollNext(prevIndex: number): void {
const start = prevIndex + this.state.chunkSize;
const end = start + this.state.startIndex - prevIndex;
const container = this.dc.instance._vcr as ViewContainerRef;

for (let i = start; i < end && this.igxForOf[i] !== undefined; i++) {
const embView = this._embeddedViews.shift();
this.scrollFocus(embView.rootNodes.find(node => node.nodeType === Node.ELEMENT_NODE)
|| embView.rootNodes[0].nextElementSibling);
const view = container.detach(0);

this.updateTemplateContext(embView.context, i);
container.insert(view);
this._embeddedViews.push(embView);
}
}

/**
* @hidden
* The function applies an optimized state change for scrolling up/left employing context change with view rearrangement
*/
protected moveApplyScrollPrev(prevIndex: number): void {
const container = this.dc.instance._vcr as ViewContainerRef;
for (let i = prevIndex - 1; i >= this.state.startIndex && this.igxForOf[i] !== undefined; i--) {
const embView = this._embeddedViews.pop();
this.scrollFocus(embView.rootNodes.find(node => node.nodeType === Node.ELEMENT_NODE)
|| embView.rootNodes[0].nextElementSibling);
const view = container.detach(container.length - 1);

this.updateTemplateContext(embView.context, i);
container.insert(view, 0);
this._embeddedViews.unshift(embView);
}
}

/**
* @hidden
*/
protected getContextIndex(input) {
return this.isRemote ? this.state.startIndex + this.igxForOf.indexOf(input) : this.igxForOf.indexOf(input);
}

/**
* @hidden
* Function which updates the passed context of an embedded view with the provided index
* from the view container.
* Often, called while handling a scroll event.
*/
protected updateTemplateContext(context: any, index: number = 0): void {
context.$implicit = this.igxForOf[index];
context.index = this.getContextIndex(this.igxForOf[index]);
context.count = this.igxForOf.length;
}

/**
* @hidden
* The function applies an optimized state change through context change for each view
Expand All @@ -879,12 +940,28 @@ export class IgxForOfDirective<T> implements OnInit, OnChanges, DoCheck, OnDestr
let j = 0;
const endIndex = this.state.startIndex + this.state.chunkSize;
for (let i = this.state.startIndex; i < endIndex && this.igxForOf[i] !== undefined; i++) {
const input = this.igxForOf[i];
const embView = this._embeddedViews[j++];
const cntx = (embView as EmbeddedViewRef<any>).context;
cntx.$implicit = input;
cntx.index = this.getContextIndex(input);
cntx.count = this.igxForOf.length;
this.updateTemplateContext(embView.context, i);
}
}

/**
* @hidden
* @internal
*
* Clears focus inside the virtualized container on small scroll swaps.
*/
protected scrollFocus(node?: HTMLElement): void {
const activeElement = document.activeElement as HTMLElement;

// Remove focus in case the the active element is inside the view container.
// Otherwise we hit an exception while doing the 'small' scrolls swapping.
// For more information:
//
// https://developer.mozilla.org/en-US/docs/Web/API/Node/removeChild
// https://bugs.chromium.org/p/chromium/issues/detail?id=432392
if (node && node.contains(document.activeElement)) {
activeElement.blur();
}
}

Expand Down Expand Up @@ -950,12 +1027,8 @@ export class IgxForOfDirective<T> implements OnInit, OnChanges, DoCheck, OnDestr
endIndex = this.igxForOf.length;
}
for (let i = startIndex; i < endIndex && this.igxForOf[i] !== undefined; i++) {
const input = this.igxForOf[i];
const embView = embeddedViewCopy.shift();
const cntx = (embView as EmbeddedViewRef<any>).context;
cntx.$implicit = input;
cntx.index = this.getContextIndex(input);
cntx.count = this.igxForOf.length;
this.updateTemplateContext(embView.context, i);
}
if (prevChunkSize !== this.state.chunkSize) {
this.onChunkLoad.emit(this.state);
Expand Down Expand Up @@ -1612,12 +1685,8 @@ export class IgxGridForOfDirective<T> extends IgxForOfDirective<T> implements On
}

for (let i = startIndex; i < endIndex && this.igxForOf[i] !== undefined; i++) {
const input = this.igxForOf[i];
const embView = embeddedViewCopy.shift();
const cntx = (embView as EmbeddedViewRef<any>).context;
cntx.$implicit = input;
cntx.index = this.getContextIndex(input);
cntx.count = this.igxForOf.length;
this.updateTemplateContext(embView.context, i);
}
if (prevChunkSize !== this.state.chunkSize) {
this.onChunkLoad.emit(this.state);
Expand Down
8 changes: 4 additions & 4 deletions projects/igniteui-angular/src/lib/grids/cell.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,21 @@
<ng-template #inlineEditor let-cell="cell">
<ng-container *ngIf="column.dataType === 'string'">
<igx-input-group displayDensity="compact">
<input igxInput [value]="editValue" (input)="editValue = $event.target.value" [igxFocus]="focused" />
<input igxInput [value]="editValue" (input)="editValue = $event.target.value" [igxFocus]="true" />
</igx-input-group>
</ng-container>
<ng-container *ngIf="column.dataType === 'number'">
<igx-input-group displayDensity="compact">
<input igxInput [value]="editValue" (input)="editValue = $event.target.value" [igxFocus]="focused" type="number">
<input igxInput [value]="editValue" (input)="editValue = $event.target.value" [igxFocus]="true" type="number">
</igx-input-group>
</ng-container>
<ng-container *ngIf="column.dataType === 'boolean'">
<igx-checkbox (change)="editValue = $event.checked" [value]="editValue" [checked]="editValue"
[igxFocus]="focused" [disableRipple]="true"></igx-checkbox>
[igxFocus]="true" [disableRipple]="true"></igx-checkbox>
</ng-container>
<ng-container *ngIf="column.dataType === 'date'">
<igx-date-picker [style.width.%]="100" [outlet]="grid.outletDirective" mode="dropdown"
[locale]="grid.locale" [(value)]="editValue" [igxFocus]="focused" [labelVisibility]="false">
[locale]="grid.locale" [(value)]="editValue" [igxFocus]="true" [labelVisibility]="false">
</igx-date-picker>
</ng-container>
</ng-template>
Expand Down
2 changes: 1 addition & 1 deletion projects/igniteui-angular/src/lib/grids/cell.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ export class IgxGridCellComponent implements OnInit, OnChanges, OnDestroy {
}
}
crud.end();
this.grid.tbody.nativeElement.focus();
this.grid.tbody.nativeElement.focus({ preventScroll: true });
this.grid.notifyChanges();
crud.begin(this);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
ColumnEditablePropertyTestComponent
} from '../../test-utils/grid-samples.spec';
import { DebugElement } from '@angular/core';
import { setupGridScrollDetection } from '../../test-utils/helper-utils.spec';

const DEBOUNCETIME = 30;
const CELL_CSS_CLASS = '.igx-grid__td';
Expand Down Expand Up @@ -382,31 +381,35 @@ describe('IgxGrid - Cell Editing #grid', () => {
});

it('When cell in editMode and try to navigate with `ArrowUp` - focus should remain over the input.', (async () => {
let cell = grid.getCellByColumn(0, 'firstName' );
UIInteractions.simulateClickAndSelectEvent(cell);
fixture.detectChanges();
GridFunctions.simulateGridContentKeydown(fixture, 'ArrowDown', false, false, true);
await wait(100);
const verticalScroll = grid.verticalScrollContainer.getScroll();
let expectedScroll;
let cellElem;
GridFunctions.scrollTop(grid, 1000);
await wait(500);
fixture.detectChanges();

cell = grid.getCellByColumn(8, 'firstName' );
UIInteractions.simulateDoubleClickAndSelectEvent(cell);
await wait(DEBOUNCETIME);
const testCells = grid.getColumnByName('firstName').cells;
const cell = testCells[testCells.length - 1];
cellElem = cell.nativeElement;

cellElem.dispatchEvent(new Event('focus'));
cellElem.dispatchEvent(new MouseEvent('dblclick'));
fixture.detectChanges();
await wait(50);

const inputElem: HTMLInputElement = document.activeElement as HTMLInputElement;
let inputElem: HTMLInputElement = document.activeElement as HTMLInputElement;
expect(cell.editMode).toBeTruthy();
expect(cell.nativeElement.classList.contains(CELL_CLASS_IN_EDIT_MODE)).toBe(true);
const expectedScroll = grid.verticalScrollContainer.getScroll().scrollTop;
expect(cellElem.classList.contains(CELL_CLASS_IN_EDIT_MODE)).toBe(true);
expectedScroll = verticalScroll.scrollTop;

UIInteractions.triggerKeyDownEvtUponElem('ArrowUp', inputElem, true);
await wait(DEBOUNCETIME);
fixture.detectChanges();
await wait(DEBOUNCETIME);

cell = grid.getCellByColumn(8, 'firstName' );
inputElem = document.activeElement as HTMLInputElement;
expect(cell.editMode).toBeTruthy();
expect(cell.nativeElement.classList.contains(CELL_CLASS_IN_EDIT_MODE)).toBe(true);
expect(grid.verticalScrollContainer.getScroll().scrollTop).toBe(expectedScroll);
expect(cellElem.classList.contains(CELL_CLASS_IN_EDIT_MODE)).toBe(true);
expect(verticalScroll.scrollTop).toBe(expectedScroll);
}));

it('When cell in editMode and try to navigate with `ArrowRight` - focus should remain over the input.', (async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1373,7 +1373,7 @@ describe('IgxGrid - Cell selection #grid', () => {
GridSelectionFunctions.verifyCellSelected(cell);
expect(grid.selectedCells.length).toBe(1);

let row = grid.getRowByIndex(3);
const row = grid.getRowByIndex(3);
expect(row instanceof IgxGridGroupByRowComponent).toBe(true);
expect(row.focused).toBe(true);

Expand All @@ -1399,7 +1399,6 @@ describe('IgxGrid - Cell selection #grid', () => {
await wait(100);
fix.detectChanges();

row = grid.getRowByIndex(3);
expect(selectionChangeSpy).toHaveBeenCalledTimes(2);
expect(grid.selectedCells.length).toBe(4);
expect(row instanceof IgxGridGroupByRowComponent).toBe(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ describe('IgxGrid Master Detail #grid', () => {
});

it('Should navigate down through a detail view partially out of view by scrolling it so it becomes fully visible.', async() => {
let row = grid.getRowByIndex(4) as IgxGridRowComponent;
const row = grid.getRowByIndex(4) as IgxGridRowComponent;
const targetCellElement = grid.getCellByColumn(4, 'ContactName');
UIInteractions.simulateClickAndSelectEvent(targetCellElement);
fix.detectChanges();
Expand All @@ -383,7 +383,6 @@ describe('IgxGrid Master Detail #grid', () => {
await wait(DEBOUNCETIME);
fix.detectChanges();

row = grid.getRowByIndex(4) as IgxGridRowComponent;
const detailRow = GridFunctions.getMasterRowDetail(row);
GridFunctions.verifyMasterDetailRowFocused(detailRow);
expect(GridFunctions.elementInGridView(grid, detailRow)).toBeTruthy();
Expand All @@ -396,7 +395,7 @@ describe('IgxGrid Master Detail #grid', () => {
await wait(DEBOUNCETIME);
fix.detectChanges();

let row = grid.getRowByIndex(6) as IgxGridRowComponent;
const row = grid.getRowByIndex(6) as IgxGridRowComponent;
const targetCellElement = grid.getCellByColumn(6, 'ContactName');
UIInteractions.simulateClickAndSelectEvent(targetCellElement);
fix.detectChanges();
Expand All @@ -408,7 +407,6 @@ describe('IgxGrid Master Detail #grid', () => {
await wait(DEBOUNCETIME);
fix.detectChanges();

row = grid.getRowByIndex(6) as IgxGridRowComponent;
const detailRow = GridFunctions.getMasterRowDetail(row);
GridFunctions.verifyMasterDetailRowFocused(detailRow);
expect(GridFunctions.elementInGridView(grid, detailRow)).toBeTruthy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { By } from '@angular/platform-browser';
import { IgxHierarchicalRowComponent } from './hierarchical-row.component';
import { setupHierarchicalGridScrollDetection } from '../../test-utils/helper-utils.spec';
import { GridFunctions } from '../../test-utils/grid-functions.spec';
import { IGridCellEventArgs } from '../grid';
import { IgxGridCellComponent, IGridCellEventArgs } from '../grid';
import { IgxChildGridRowComponent } from './child-grid-row.component';

const DEBOUNCE_TIME = 60;
Expand Down Expand Up @@ -610,14 +610,17 @@ describe('IgxHierarchicalGrid Basic Navigation #hGrid', () => {
hierarchicalGrid.getColumnByName('ID').hidden = true;
await wait(50);
fixture.detectChanges();
hierarchicalGrid.navigateTo(2);
await wait(DEBOUNCE_TIME);
fixture.detectChanges();

const cell = hierarchicalGrid.getCellByColumn(0, 'ChildLevels');
const cell = hierarchicalGrid.getCellByColumn(2, 'ChildLevels');
UIInteractions.simulateDoubleClickAndSelectEvent(cell);
fixture.detectChanges();
await wait(DEBOUNCE_TIME);
fixture.detectChanges();

UIInteractions.triggerEventHandlerKeyDown('tab', baseHGridContent, false, true, false);
UIInteractions.triggerKeyDownEvtUponElem('tab', cell.nativeElement, true, false, true);
await wait(DEBOUNCE_TIME);
fixture.detectChanges();
const activeEl = document.activeElement;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ it('should update scroll height after expanding/collapsing row in a nested child
expect(childRowComponent.index).toBe(4);

hierarchicalGrid.verticalScrollContainer.scrollNext();
await wait(200);
await wait(100);
fixture.detectChanges();
childRowComponent = fixture.debugElement.query(By.css('igx-child-grid-row')).componentInstance;
expect(childRowComponent.rowData.rowID).toBe('3');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,6 @@ export class IgxGridCRUDService {
}

begin(cell): void {
// this is necessary beacuse when the cell enters edit mode the focus should be moved to the edit template
// if we constantly retrigger the focus over the edit template we broke the scrolling experience in the hierarchical grid
// This fix should be removed, when the issue #7219 is resolved
cell.focused = true;
const newCell = this.createCell(cell);
newCell.primaryKey = this.primaryKey;
const args = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,6 @@ describe('IgxTreeGrid - Key Board Navigation #tGrid', () => {
UIInteractions.triggerEventHandlerKeyDown('ArrowDown', gridContent);
await wait(DEBOUNCETIME);
fix.detectChanges();
cell = treeGrid.getCellByColumn(i, 'ID');
TreeGridFunctions.verifyTreeGridCellSelected(treeGrid, cell, false);
cell = treeGrid.getCellByColumn(i + 1, 'ID');
TreeGridFunctions.verifyTreeGridCellSelected(treeGrid, cell);
Expand All @@ -432,7 +431,6 @@ describe('IgxTreeGrid - Key Board Navigation #tGrid', () => {
UIInteractions.triggerEventHandlerKeyDown('ArrowUp', gridContent);
await wait(DEBOUNCETIME);
fix.detectChanges();
cell = treeGrid.getCellByColumn(i, 'ID');
TreeGridFunctions.verifyTreeGridCellSelected(treeGrid, cell, false);
cell = treeGrid.getCellByColumn(i - 1, 'ID');
TreeGridFunctions.verifyTreeGridCellSelected(treeGrid, cell);
Expand Down
Loading

0 comments on commit 720f0e8

Please sign in to comment.