Filter widgets (controls) do not show up if filtering is enabled dynamically

[yurigenin]

Here's a plunker based on the filtering tutorial (click here). Press 'Toggle Filtering' button and nothing happens.

Basically, if you have selection header present and filtering is disabled initially and you get all the column definitions asynchronously and then enabled filtering it does not recalculate the header heights correctly even though the elements are there in the DOM with right heights but its container has a normal (non-filter) height.

This is pretty major for us since we always load column definitions from the db.

[PaulL1]

It's actually also doing it even if filtering is enabled by default. The 401 tutorial has filtering enabled by default and also a selection row header, and doesn't have this issue.

I presume the difference is that the columnDefs are loaded later in this example. Perhaps a dummy column def could be used that is then later overwritten - as a workaround?

If I do that: http://plnkr.co/edit/GPILOjQh9HDGOyQ8ngHm?p=preview, it works better, but now the header height doesn't collapse when the filtering is removed.

[yurigenin]

Yes, this is exactly what I had to do: have a dummy column initially before all other columns are available. It works, but do I like it? Definitely not. Thinking about other users stumbling on this behavior. But cannot complain much. Free product after all and have no time proposing a fix. Grid otherwise looks great except a couple of other issues I reported like flickering due to using static positioning for div containers. You moved it to enhancement category. Hmm, pretty major bug to me.

[c0bra]

@PaulL1 dang, this was fixed with #2800 but I guess it's broken since.

[c0bra]

@PaulL1 @yurigenin OK it works for me when I remove ui-grid-selection. So something in that feature is breaking this.

[yurigenin]

Header container height is calculated only once based on any of the headers having a filter. Since the selection header does not and it is the only one available initially the entire container height is set to its height. Even if you add columns with filters later it does not change anything.

On Mar 30, 2015, at 9:09 AM, Brian Hann notifications@github.com wrote:

@PaulL1 @yurigenin OK it works for me when I remove ui-grid-selection. So something in that feature is breaking this.

—
Reply to this email directly or view it on GitHub.

[c0bra]

@yurigenin Ah, thanks for the diagnosis.

I think what needs to happen is have the header recalculation ignore any header heights that are set explicitly when calculating the "minimum" required height. That lets any non-explicit headers set the height and any explicit ones can shrink.

[PaulL1]

The explicit header height had a bit of a smell about it for me. It felt like we dynamically calculate then store on the header, next time we calculate we can then only go up not down. If we're going to ignore the stored value then why store it at all?

Sent from my iPhone

On 31 Mar 2015, at 7:06 am, Brian Hann notifications@github.com wrote:

@yurigenin Ah, thanks for the diagnosis.

I think what needs to happen is have the header recalculation ignore any header heights that are set explicitly when calculating the "minimum" required height. That lets any non-explicit headers set the height and any explicit ones can shrink.

—
Reply to this email directly or view it on GitHub.

[c0bra]

If you have two headers, one with filtering and one without, then the one without will be shorter than the one with filtering UNLESS you give it an explicit height.

In order to do that you have to calculate the rendered heights, take the max, and set it as the explicit height on any headers shorter than the max.

I am just changing the calculation so any explicit heights are left out of the "max" check.

[yurigenin]

The original problem was that headers did not grow in height. Will your fix take care of it as well?

From: Brian Hann notifications@github.com
To:angular-ui/ng-grid ng-grid@noreply.github.com
CC:yurigenin yurig@netiq.com
Date: 3/30/2015 1:17 PM
Subject: Re: [ng-grid] Filter widgets (controls) do not show up if filtering is enabled dynamically (#3138)

If you have two headers, one with filtering and one without, then the one without will be shorter than the one with filtering UNLESS you give it an explicit height.

In order to do that you have to calculate the rendered heights, take the max, and set it as the explicit height on any headers shorter than the max.

I am just changing the calculation so any explicit heights are left out of the "max" check.

—
Reply to this email directly or view it on GitHub.

[yurigenin]

Please reopen. The plunker in 3138 shows that it does not work. Even worse, the headers now are half their height. Totally broken.

From: Brian Hann notifications@github.com
To:angular-ui/ng-grid ng-grid@noreply.github.com
CC:yurigenin yurig@netiq.com
Date: 3/30/2015 1:19 PM
Subject: Re: [ng-grid] Filter widgets (controls) do not show up if filtering is enabled dynamically (#3138)

Closed #3138 via 7c5cdca.

—
Reply to this email directly or view it on GitHub.

[yurigenin]

This still does not work: http://plnkr.co/edit/3rY4J0GjVCuouuwFske0?p=preview

[yurigenin]

With all due respect, why did you close it before the submitter (me) had an opportunity to verify? I obviously cannot verify since nothing is working now due to build issues. What's the process anyway? Don't I have the last word on whether it is fixed or not?

Thank you,

Yuri

From: Brian Hann notifications@github.com
To:angular-ui/ng-grid ng-grid@noreply.github.com
CC:yurigenin yurig@netiq.com
Date: 3/30/2015 1:19 PM
Subject: Re: [ng-grid] Filter widgets (controls) do not show up if filtering is enabled dynamically (#3138)

Closed #3138 via 7c5cdca.

—
Reply to this email directly or view it on GitHub.

[c0bra]

It's not fixed because the the build process failed because the tests do not pass in IE , and I am working on it.

[yurigenin]

I appreciate it. I just wanted to know the process you are following. So you basically can close even before somebody who submitted it verifies it? It would be reasonable to give at least a week to verify, in my opinion...

It's not fixed because the the build process failed because the tests do not pass in IE , and I am working on it.

—
Reply to this email directly or view it on GitHub.

[c0bra]

@yurigenin Github automatically closes tickets based on text in the commit message. It's good to be able to link commits directly to the issues they fix.

There were a couple problems in this case:

1. I pushed the fix to master rather than doing it in a PR, which mean it closed your issue before Travis ran the tests on all the browsers. If I had put it in a PR first then that would have caught the issue.
2. The change DID fix the problem, but incompletely because IE was doing things differently and I had to account for that.

Part of the problem we have is that we do try to get feedback before closing things but often as not people never respond and we're left waiting and eventually just have to close the issue after a while. This leaves lots and lots of open tickets which makes an administration nightmare for a very, very small team.

I'm sorry your issue got closed before you had a chance to confirm the fix, but we are trying our best to make sure things go smoothly for everyone, on all sides!

[yurigenin]

I just checked the plunker attached to that issue but it is still not fixed. When you click 'Toggle Filtering' button the filter boxes do not show up so please reopen this issue. Build issue is resolved, though.

@yurigenin Github automatically closes tickets based on text in the commit message. It's good to be able to link commits directly to the issues they fix.

There were a couple problems in this case:
I pushed the fix to master rather than doing it in a PR, which mean it closed your issue before Travis ran the tests on all the browsers. If I had put it in a PR first then that would have caught the issue.
The change DID fix the problem, but incompletely because IE was doing things differently and I had to account for that.

Part of the problem we have is that we do try to get feedback before closing things but often as not people never respond and we're left waiting and eventually just have to close the issue after a while. This leaves lots and lots of open tickets which makes a administration nightmare for a very, very small team.

I'm sorry your issue got closed before you had a chance to confirm the fix, but we are trying our best to make sure things go smoothly for everyone, on all sides!

—
Reply to this email directly or view it on GitHub.

[c0bra]

That's because angular-touch isn't included in that plunker. Separate issue.

[yurigenin]

Can you explain? What does it have to do with angular-touch? This is just a desktop example. Why would I need angular-touch for that?

That's because angular-touch isn't included in that plunker. Separate issue.

—
Reply to this email directly or view it on GitHub.

[beffjarker]

Did anyone ever fix whatever in ui-grid-selection was causing this issue? I can't remove ui-grid-selection as I need it, but the filters are not showing for me even though it's enabled. height is not explicitly set.