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

igx-grid - calculation of grid height has been broken when enabling/disabling filter row after updating to v7.0.4. #3363

Closed
tkiryu opened this issue Dec 10, 2018 · 9 comments

Comments

@tkiryu
Copy link

tkiryu commented Dec 10, 2018

Description

I checked the fix for #3255 and found that calculation of grid height has been broken when enabling/disabling filter row after updating to v7.0.4.

  • igniteui-angular version: 7.0.4
  • browser: All

Steps to reproduce

Same as #3255.

  1. Run the attached sample.
  2. Click "Enable Filtering" button.
  3. Click "Filter" icon on any column.
  4. Click "Disable Filtering" button.

Result

  • At step 1
    There is no problem.
    image

  • After step 2
    The body height of grid shrinks.
    image

  • After step 4
    The body of grid gets zero height.
    image

Expected result

The body height of grid should not shrink as before.

Attachments

Same as #3255.
igx-grid.zip

@bkulov
Copy link
Contributor

bkulov commented Dec 10, 2018

This could also be related to #3290

@bkulov
Copy link
Contributor

bkulov commented Dec 10, 2018

@tkiryu The sample you've attached is using Angular 6 and IgniteUI for Angular 6.2.0. In that version the bug you've described is not reproducible.
The bug you've described is reproducible in IgniteuUI for Angular 7.0.4 but it's not related to the filtering. Filtering itself is working properly when enabling/disabling it in the grid.
The actual problem here is that when there is no height set on the grid (which is the case in this bug), the grid tries to calculate it.

I'll change the title of this issue a little bit and will reassign this issue to the people who wrote the height logic.

I'm also attaching the proper sample converted to IgniteUI 7.0.4.
igx-grid.zip

@bkulov bkulov changed the title [Related to #3255] igx-grid - calculation of grid height has been broken when enabling/disabling filter row after updating to v7.0.4. igx-grid - calculation of grid height has been broken when enabling/disabling filter row after updating to v7.0.4. Dec 10, 2018
@bkulov bkulov assigned ChronosSF and mpavlinov and unassigned bkulov and SAndreeva Dec 10, 2018
@bkulov
Copy link
Contributor

bkulov commented Dec 10, 2018

@tkiryu There is something else wrong with the sample as the grid doesn't have any styles. Not sure if this affects the current behavior.

@tkiryu
Copy link
Author

tkiryu commented Dec 19, 2018

@bkulov

The sample you've attached is using Angular 6 and IgniteUI for Angular 6.2.0.

Oh, I'm sorry about that.

The bug you've described is reproducible in IgniteuUI for Angular 7.0.4 but it's not related to the filtering. Filtering itself is working properly when enabling/disabling it in the grid.
The actual problem here is that when there is no height set on the grid (which is the case in this bug), the grid tries to calculate it.

Thank you for investigating. I get your point.

I think calculation of grid height is fragile. I update to the latest igniteui-angular as soon as it is released and often encounter such an issue. I hope that the height calculation logic will be more stable.

@bkulov
Copy link
Contributor

bkulov commented Dec 19, 2018

@tkiryu I've assigned the bug to @ChronosSF who has more knowledge in the area.

@tkiryu
Copy link
Author

tkiryu commented Dec 21, 2018

@bkulov , thanks!

@tkiryu
Copy link
Author

tkiryu commented Dec 21, 2018

When igx-grid doesn't have scrollbar, this issue occurs.

When igx-grid has scrollbar, this issue won't occur.

Since this sample has a small amount of data(10 rows), which leads to no scrollbar, I found the issue by chance.

@kdinev
Copy link
Member

kdinev commented Feb 22, 2019

@tkiryu Something seems quite wrong with the theming in these screenshots. Is this from the theming, or is it the sample?

The grid height calculations are based on the record heights, which are considered before the grid is rendered, because virtualization doesn't work if content is already rendered. The record heights seem unnatural in this sample, even for compact display density, so this may be affecting your rendering. Again, I think this is related to the theme.

@MayaKirova
Copy link
Contributor

Duplicate of #4458, which is already fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants