Skip to content

Commit

Permalink
fix(datagrid): nested in accordion columns have wrong width (#1494)
Browse files Browse the repository at this point in the history
## PR Checklist

Please check if your PR fulfills the following requirements:

- [x] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)
- [ ] If applicable, have a visual design approval

## PR Type

What kind of change does this PR introduce?

- [x] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Documentation content changes
- [ ] Other... Please describe:

## What is the current behavior?
Nested in accordion datagrid columns have wrong width.

![image](https://github.com/user-attachments/assets/ef3f2f2a-53ad-4673-815c-0c18c421d5c9)

Issue Number: CDE-2208

## What is the new behavior?
Nested in accordion datagrid columns have proper width.

![image](https://github.com/user-attachments/assets/dd443cd8-67fd-4efe-99ee-e101d08f3970)

## Does this PR introduce a breaking change?

- [ ] Yes
- [x] No

## Other information

---------

Co-authored-by: GitHub <noreply@github.com>
  • Loading branch information
valentin-mladenov and web-flow authored Jul 31, 2024
1 parent 2f2cca1 commit 9a521e3
Show file tree
Hide file tree
Showing 21 changed files with 146 additions and 25 deletions.
40 changes: 39 additions & 1 deletion .storybook/stories/datagrid/datagrid-detail.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export default {
elements,
detailContentType: 'json',
showLongContent: false,
showLongUninterruptedContent: false,
highlight: true,
singleSelectable: false,
multiSelectable: false,
Expand All @@ -57,8 +58,18 @@ const longContentElement: Element = {
electronegativity: 1.1,
};

const longUninterruptedContentElement: Element = {
name: 'aReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongUninterruptedContent',
symbol: 'Ac',
number: 89,
electronegativity: 1.1,
};

const DetailTemplate: StoryFn = args => {
args.elements = args.showLongContent ? [longContentElement, ...args.elements] : args.elements;
args.elements = args.showLongUninterruptedContent
? [longUninterruptedContentElement, ...args.elements]
: args.elements;

return {
template: `
Expand All @@ -76,7 +87,7 @@ const DetailTemplate: StoryFn = args => {
${args.singleSelectable ? '[clrDgSingleSelected]="true"' : ''}
[ngClass]="{ 'datagrid-compact': compact }"
>
<clr-dg-column [style.width.px]="250">
<clr-dg-column ${args.showLongUninterruptedContent ? '' : '[style.width.px]="250"'}>
<ng-container ${args.hidableColumns ? '*clrDgHideableColumn' : ''}>Name</ng-container>
</clr-dg-column>
<clr-dg-column [style.width.px]="250">
Expand Down Expand Up @@ -169,3 +180,30 @@ export const OpenDetail: StoryObj = {
height: 500,
},
};

export const OpenLongContentDetail: StoryObj = {
render: DetailTemplate,
play({ canvasElement }: StoryContext) {
canvasElement.querySelector<HTMLButtonElement>('button.datagrid-detail-caret-button').click();

removeFocusOutline({ canvasElement });
},
args: {
detailContentType: 'datagrid',
showLongContent: true,
},
};

// Open uninterrupted content cell regression test for nested datagrid CDE-2208.
export const OpenLongUninterruptedContentDetail: StoryObj = {
render: DetailTemplate,
play({ canvasElement }: StoryContext) {
canvasElement.querySelector<HTMLButtonElement>('button.datagrid-detail-caret-button').click();

removeFocusOutline({ canvasElement });
},
args: {
detailContentType: 'datagrid',
showLongUninterruptedContent: true,
},
};
18 changes: 14 additions & 4 deletions projects/angular/src/data/datagrid/datagrid.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,10 @@ export default function (): void {
expect(refreshed).toBe(true);
});

it('allows to manually resize the datagrid', function () {
it('allows to manually resize the datagrid', fakeAsync(function () {
context.detectChanges();
tick();

const organizer: DatagridRenderOrganizer = context.getClarityProvider(DatagridRenderOrganizer);
let resizeSteps = 0;
organizer.renderStep.subscribe(() => {
Expand All @@ -513,7 +516,7 @@ export default function (): void {
expect(resizeSteps).toBe(0);
context.clarityDirective.resize();
expect(resizeSteps).toBe(5);
});
}));
});

describe('Template API', function () {
Expand Down Expand Up @@ -1348,10 +1351,17 @@ export default function (): void {
});

// Tests if manual style="width: 123px" was applied and not overridden during the calculation from the above test.
it('column width manual setting is applied', function () {
it('column width manual setting is applied', fakeAsync(function () {
context.detectChanges();
tick();

expect(context.clarityElement.querySelector('.datagrid-column').clientWidth).toBe(123);

context.detectChanges();
tick();

expect(context.clarityElement.querySelector('.datagrid-column').getAttribute('style')).toBe('width: 123px;');
});
}));
});

describe('detail pane and track by', function () {
Expand Down
33 changes: 25 additions & 8 deletions projects/angular/src/data/datagrid/render/header-renderer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { Component, DebugElement } from '@angular/core';
import { fakeAsync, tick } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { BehaviorSubject } from 'rxjs';

Expand Down Expand Up @@ -175,8 +176,12 @@ export default function (): void {
columnSeparator.hideTracker();
};

beforeEach(function () {
beforeEach(fakeAsync(function () {
context = this.create(ClrDatagrid, HeaderResizeTestComponent);

context.detectChanges();
tick();

columnHeader1DebugElement = context.fixture.debugElement.queryAll(By.directive(DatagridHeaderRenderer))[0];
columnHeader2DebugElement = context.fixture.debugElement.queryAll(By.directive(DatagridHeaderRenderer))[1];
columnHeader3DebugElement = context.fixture.debugElement.queryAll(By.directive(DatagridHeaderRenderer))[2];
Expand All @@ -199,7 +204,7 @@ export default function (): void {
columnHeader3ColumnSeparatorDebugElement = context.fixture.debugElement.queryAll(
By.directive(ClrDatagridColumnSeparator)
)[2];
});
}));

it('each header should have min-width', function () {
expect(columnHeader1ResizerService.minColumnWidth).toBe(96);
Expand All @@ -222,17 +227,21 @@ export default function (): void {
expect(column4InitialWidth).toBeGreaterThan(columnHeader4ResizerService.minColumnWidth);
});

it('expands other flexible headers if header width shrinks', function () {
it('expands other flexible headers if header width shrinks', fakeAsync(function () {
const resizeBy = -20;
emulateResizeOnColumn(resizeBy, columnHeader1ColumnSeparatorDebugElement.componentInstance);

context.detectChanges();
tick();

expect(widthOf(columnHeader1Element)).toBe(column1InitialWidth + resizeBy);
expect(widthOf(columnHeader2Element)).toBeGreaterThan(column2InitialWidth);
expect(widthOf(columnHeader3Element)).toBe(
column3InitialWidth,
`A strict width shouldn't change when other header's width changes`
);
expect(widthOf(columnHeader4Element)).toBeGreaterThan(column4InitialWidth);
});
}));

it('resized header should have fixed width class', function () {
expect(columnHeader1Element.classList.contains(STRICT_WIDTH_CLASS)).toBeFalse();
Expand All @@ -253,29 +262,37 @@ export default function (): void {
expect(widthOf(columnHeader4Element)).toBeLessThan(column4InitialWidth);
});

it("shouldn't shrink flexible headers below their min-width if header width expands by large amount", function () {
it("shouldn't shrink flexible headers below their min-width if header width expands by large amount", fakeAsync(function () {
const resizeBy = 1000;
emulateResizeOnColumn(resizeBy, columnHeader1ColumnSeparatorDebugElement.componentInstance);

context.detectChanges();
tick();

expect(widthOf(columnHeader1Element)).toBe(column1InitialWidth + resizeBy);
expect(widthOf(columnHeader2Element)).toBe(120);
expect(widthOf(columnHeader3Element)).toBe(
200,
`A strict width shouldn't change when other header's width changes`
);
expect(widthOf(columnHeader4Element)).toBe(96);
});
}));

it('gives header its min-width if a user tried to drag too much to left', function () {
const resizeBy = -1000;
emulateResizeOnColumn(resizeBy, columnHeader1ColumnSeparatorDebugElement.componentInstance);
expect(widthOf(columnHeader1Element)).toBe(96);
});

it('emits new header width once resizing ends', function () {
it('emits new header width once resizing ends', fakeAsync(function () {
expect(context.testComponent.newWidth).toBeUndefined();
const resizeBy = 20;
emulateResizeOnColumn(resizeBy, columnHeader3ColumnSeparatorDebugElement.componentInstance);

context.detectChanges();
tick();

expect(context.testComponent.newWidth).toBe(column3InitialWidth + resizeBy);
});
}));
});
}
33 changes: 22 additions & 11 deletions projects/angular/src/data/datagrid/render/main-renderer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { Component, ElementRef, ViewChild } from '@angular/core';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations'; // Needed to recreate issue #1084

Expand All @@ -30,7 +30,7 @@ export default function (): void {
let computeStateSpy: jasmine.Spy;
let columnsService: ColumnsService;

beforeEach(function () {
beforeEach(fakeAsync(function () {
resizeSpy = spyOn(DatagridRenderOrganizer.prototype, 'resize');
context = this.createWithOverrideDirective(
DatagridMainRenderer,
Expand All @@ -42,7 +42,10 @@ export default function (): void {
organizer = context.getClarityProvider(DatagridRenderOrganizer) as MockDatagridRenderOrganizer;
computeStateSpy = spyOn(DatagridHeaderRenderer.prototype, 'getColumnWidthState');
columnsService = context.getClarityProvider(ColumnsService);
});

context.detectChanges();
tick();
}));

it('triggers the render process on initialization', function () {
expect(resizeSpy.calls.count()).toBe(1);
Expand Down Expand Up @@ -100,37 +103,45 @@ export default function (): void {
let context: TestContext<DatagridMainRenderer, DynamicTest>;
let resizeSpy, rowsSpy: jasmine.Spy;

beforeEach(function () {
beforeEach(fakeAsync(function () {
resizeSpy = spyOn(DatagridRenderOrganizer.prototype, 'resize');
rowsSpy = spyOn(DatagridRowRenderer.prototype, 'setCellsState');
context = this.create(DatagridMainRenderer, DynamicTest);
});

it('does not trigger the render process until the rows are loaded', function () {
context.detectChanges();
tick();
}));

it('does not trigger the render process until the rows are loaded', fakeAsync(function () {
expect(resizeSpy.calls.count()).toBe(0);
context.testComponent.projected = true;
context.detectChanges();
tick();
expect(resizeSpy.calls.count()).toBe(1);
});
}));

it('ignores columns changes until the rows are loaded', function () {
it('ignores columns changes until the rows are loaded', fakeAsync(function () {
context.testComponent.secondColumn = false;
context.detectChanges();
tick();
expect(resizeSpy.calls.count()).toBe(0);
context.testComponent.projected = true;
context.detectChanges();
tick();
expect(resizeSpy.calls.count()).toBe(1);
context.testComponent.secondColumn = true;
context.detectChanges();
tick();
expect(resizeSpy.calls.count()).toBe(2);
});
}));

it('triggers the render process if the rows are given through *clrDgItems', function () {
it('triggers the render process if the rows are given through *clrDgItems', fakeAsync(function () {
expect(resizeSpy.calls.count()).toBe(0);
context.testComponent.clrDgItems = [1];
context.detectChanges();
tick();
expect(resizeSpy.calls.count()).toBe(1);
});
}));

it('tracks changes of cells', function () {
context.testComponent.clrDgItems = [0, 1, 2];
Expand Down
5 changes: 4 additions & 1 deletion projects/angular/src/data/datagrid/render/main-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,11 @@ export class DatagridMainRenderer implements AfterContentInit, AfterViewInit, Af

ngAfterViewChecked() {
if (this.shouldStabilizeColumns) {
this.stabilizeColumns();
setTimeout(() => {
this.stabilizeColumns();
}, 0);
}

if (this.shouldComputeHeight()) {
this.ngZone.runOutsideAngular(() => {
setTimeout(() => {
Expand Down
23 changes: 23 additions & 0 deletions projects/demo/src/app/accordion/accordion.demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,29 @@ <h2>Angular - multi panel</h2>
</clr-accordion-panel>
</clr-accordion>

<h2>Angular - nested datagrid</h2>

<clr-accordion>
<clr-accordion-panel>
<clr-accordion-title>Item 1</clr-accordion-title>
<clr-accordion-content *clrIfExpanded>
<clr-datagrid>
<clr-dg-column>User ID</clr-dg-column>
<clr-dg-column>Name</clr-dg-column>
<clr-dg-column>test</clr-dg-column>

<clr-dg-row *clrDgItems="let user of users">
<clr-dg-cell>{{ user.id }}</clr-dg-cell>
<clr-dg-cell> {{ user.name }} </clr-dg-cell>
<clr-dg-cell>Test</clr-dg-cell>
</clr-dg-row>

<clr-dg-footer>{{ users.length }} users</clr-dg-footer>
</clr-datagrid>
</clr-accordion-content>
</clr-accordion-panel>
</clr-accordion>

<h2>Angular - complex multi panel example (first panel {{stepOpen ? 'open' : 'closed'}})</h2>

<button (click)="disableThirdPanel = !disableThirdPanel" class="btn">
Expand Down
19 changes: 19 additions & 0 deletions projects/demo/src/app/accordion/accordion.demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,25 @@ export class AccordionDemo {
stepOpen = true;
disableThirdPanel = true;

users = [
{
id: 'id-1',
name: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
},
{
id: 'id-2',
name: 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb',
},
{
id: 'id-3',
name: 'cccccccccccccccccccccccccccccccccccccccccccccccccccccccc',
},
{
id: 'id-4',
name: 'dddddddddddddddddddddddddddddddddddddddddddddddddddddddd',
},
];

change(event) {
console.log('Accordion Changed', event);
}
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/firefox/datagrid/detail--open-detail-core-dark.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 9a521e3

Please sign in to comment.