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

Details broken when hidden #1754

Merged
merged 3 commits into from
Jun 14, 2020

Conversation

christophe-g
Copy link
Contributor

When a column is hidden , whatever is within if (index === columns.length - 1 && (this._rowDetailsTemplate || this.rowDetailsRenderer)) body is never called.

@tomivirkki
Copy link
Member

I guess this fixes #1737?

@christophe-g
Copy link
Contributor Author

@tomivirkki

I guess this fixes #1737?

Haha, likely so. I did not pay serious attention to this issue, as thought it was due to potential conflict with litElement.

@christophe-g
Copy link
Contributor Author

But pushed in relation to #1753

@tomivirkki
Copy link
Member

Also add a test case for the issue/fix here:

it('should not remove details row when a column is hidden', () => {
  grid.rowDetailsRenderer = root => root.textContent = 'row-details';
  grid.detailsOpenedItems = [grid._cache.items[0]];
  column.hidden = true;
  flushGrid(grid);
  const details = grid.shadowRoot.querySelector('#items [part~="details-cell"]')._content;
  expect(details.textContent).to.equal('row-details');
});

@christophe-g
Copy link
Contributor Author

@tomivirkki - first time someone writes tests for me ! Appreciated.

@tomivirkki
Copy link
Member

No problem :) This is quite a critical issue so the fix is also very much appreciated.

There still seems to be some white-space issue in the test file https://travis-ci.org/github/vaadin/vaadin-grid/jobs/697588806#L402

@tomivirkki tomivirkki merged commit 7eabb73 into vaadin:5.6 Jun 14, 2020
tomivirkki pushed a commit that referenced this pull request Jun 14, 2020
tomivirkki pushed a commit that referenced this pull request Jun 15, 2020
tomivirkki pushed a commit that referenced this pull request Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants