ManifestWebDesign / angular-gridster

An implementation of gridster-like widgets for Angular JS
http://manifestwebdesign.github.io/angular-gridster/
MIT License
964 stars 394 forks source link

Angular 1.3, Bootstrap Tabs (or 'display: none') and resizing #139

Open mtraynham opened 10 years ago

mtraynham commented 10 years ago

1.2.26: http://jsfiddle.net/eosrz86r/ 1.3.0: http://jsfiddle.net/82d9ejtL/

1.2.26 seems to be fine from my initial interpretation. If you switch tabs, the grid doesn't resize at all. Not resizing when hidden is actually a good thing, as it doesn't trigger redraws to the content of each item (in my case it can be expensive). In 1.3.0, if you switch tabs, the grid sizes all of it's content down to 0 when hidden. But once you switch back, it doesn't resize at all leaving a blank page.

I would expect no sizing should happen when display: none or offsetWidth: 0

This can also be seen with a simple ng-show and $interval, but it has a slightly different outcome. In Angular 1.3, you'll see a genie-flicker effect because the display: none caused a recalculation of sizes to be 0. When it's displayed again, it auto-resizes from 0 to the width of it's container. This is slightly different from the tabs because the tab does't trigger a resize on it's content. 1.2.26: http://jsfiddle.net/52wj5t0h/ 1.3.0: http://jsfiddle.net/qw7pc0cc/

Besides this, I didn't notice any other breaking changes with the upgrade. This seems to be my only hold up for migrating to 1.3 though, which is better than I expected :)

TheBosZ commented 10 years ago

These are some great reports, thanks!

I noticed that for the first set, if I resize the window, the grid pops in.

mtraynham commented 10 years ago

Thanks! You are correct, the window resize does trigger the update of all the items and they get the correct width. For performance reasons, it would probably be ideal to not update the grid sizes if hidden.

I havn't dived into the other problem of why the tab content is still hidden at first... My guess is there is probably too much going on in the single browser loop (ie. the tab becomes visible with 0 width, the grid resizes to 0 width, then the tab resizes to full width).

mtraynham commented 10 years ago

An even more simplistic example to help pinpoint the problem (i.e. removed the dependencies on jquery.resize.js). The grid no longer renders at all (unless you force window resize) when switching between display:none & display:block:

1.2.6: http://jsfiddle.net/12jakg7x/ 1.3.0: http://jsfiddle.net/2j71ap2o/

barretodavid commented 9 years ago

I have been discussing with @mtraynham this problem because it is happening to me using the latest versions of all the dependencies. When you put a gridster element inside a bootstrap-ui tab, the grid does not show up unless you manually resize the browser. It seems like the width and height of the grid its calculated when the tab still have the CSS rule display:none so the effective size is always zero. When bootstrap-ui activates the tab (a few milliseconds later), it changes the CSS rule to display:block but gridster is never notified about this.

The documentation states that we should use the jquery version of the module javascript-detect-element-resize but when you use a generator like angular-gulp who handles those dependencies for you, it loads the javascript version of the module, not the jquery one because the bower.json manifest of the module javascript-detect-element-resize points to the file detect-element-resize.js and not the recommended file jquery.resize.js. If I manually change the reference everything works just fine.

mtraynham commented 9 years ago

@barretodavid were you able to replicate this in a fiddle?

mtraynham commented 9 years ago

By the way, I tried out your recommendation of using the jquery.resize.js instead and it works appropriately. I wonder if the other implementation just doesn't catch as many resize cases... That being said, I bet the jquery one is probably less performant.

barretodavid commented 9 years ago

@mtraynham there must be something about how jsfiddle works because I wasn't able to replicate the problem there. The problem shows when working locally on my computer.

barretodavid commented 9 years ago

@mtraynham my guess is that is a race condition (using the javascript version of the resize detection), and because jsfiddle has to deal with more things than when working locally, the grid loading happens after the tab content is changed to display:block so it calculates its size correctly.

mtraynham commented 9 years ago

@barretodavid I created a test project for it: https://github.com/mtraynham/angular-gridster-151

Although the test project seems to work, my main project still does not with the detect-element-resize.js...

barretodavid commented 9 years ago

@mtraynham both of your examples works fine on my computer, the only difference is that I can see a glitch when using the jquery plugin. But I had the same problem as you describe, I still haven't manage to implement a proper solution on my main project. I still think is a race condition, maybe the jquery plugin just slows things down a bit and thus gives enough time to the tab plugin to change the display rule.

mtraynham commented 9 years ago

@barretodavid Found the problem. You can't have both the detect-element-resize.js and jquery.js on the page. I pushed a new html page to that repo that shows the issue.

https://github.com/mtraynham/angular-gridster-151/blob/master/test-detect-element-resize-with-jquery.html

Likely caused by a change to angular-gridster at some point between 0.11.7 and 0.13.2, as that was my upgrade path and it worked before...

This is sort of unfortunate, as the detect-element-resize.js is probably more performant and doesn't rely on jquery at all; nor does it have that silly flicker effect. That and I was using it else-where :). I can't easily get rid of jquery as some of my other dependencies rely on it...

As it seems to work without jquery.js, maybe we can get @danomatic or @TheBosZ to chime in and see if there is something that can be done. There was already a big push to remove jquery dragging from this repo, so hopefully we can just accept that we'll always use the detect-element-resize.js script, especially because that is main script in that repo...

If they do chime in, here is the test project I've been using: https://github.com/mtraynham/angular-gridster-151

I think this is still caused by tabs/display:none, as I couldn't reproduce the problem outside of tabs and I've updated the code to show that it works outside of tabs.

mtraynham commented 9 years ago

@barretodavid So a little bit more digging. It works on 0.11.7, but not 0.12.0+. https://github.com/ManifestWebDesign/angular-gridster/commit/e650ad2e146f98aaf682aa79af761b7acb50d44a is the commit that breaks it.

Likely because https://api.jquery.com/resize/ is a function, even though we aren't using the version obtained by the detect-element-resize package.

mtraynham commented 9 years ago

My thinking is it might be good to remove this if clause: https://github.com/ManifestWebDesign/angular-gridster/blob/master/src/angular-gridster.js#L747

Because both detect-element-resize.js and jquery.resize.js append window.addResizeListener to the scope, it's almost a pointless check and when you have these incorrect scripts added, you get bad functionality.

There is still the random flicker of sizing, which may be a separate issue as it's also caused when just using the jquery versions... Don't see it with the detect-element-resize only.

barretodavid commented 9 years ago

@mtraynham let me see if I understand correctly. The purpose of that if clause was to know if the jquery.resize.js plugin was present but it is being triggered always, even when detect-element-resize.js was being used?

mtraynham commented 9 years ago

Correct. jQuery has it's own resize function (as noted above). The jquery.resize.js script overrides that function. So, if you have either jQuery.js or jquery.resize.js on the page, it falls into that if block.

As noted, the standard jQuery resize handling is not what we want.

This check is pointless though, as both scripts from javascript-detect-element-resize append the window.addResizeListener, so it could just always use that instead. Creating a PR as we speak :)

mtraynham commented 9 years ago

In regards to the flicker, I would assume that an initial rendering of a gridster-item should have opacity: 0, then fade in to opacity: 1. Although, I believe this may be counter-acted by the display: none to display: visible switch on the tabs.

mtraynham commented 9 years ago

@barretodavid The flicker can be fixed based on the solution obtained from here. Although, I don't know of the performance ramifications involved in this change... I imagine display: none is better for a large amount of tabs.

In your own CSS:

.tab-content > .tab-pane {
    display: block;
    height: 0;
    overflow-y: hidden;
}

.tab-content > .active {
    height: auto;
}
barretodavid commented 9 years ago

@mtraynham great, thank you!

mtraynham commented 9 years ago

I'm going to leave this open still... I'd much rather see a fix for handling of display: none parents that offers better performance. But hopefully that should hold you off for the time being.

danomatic commented 9 years ago

Please try the latest version and let me know if it clears up some of these issues.

mtraynham commented 9 years ago

@danomatic Thanks for accepting that PR. The jquery+javascript-detect-element issue does look resolved.

I still see the flicker with jQuery. My theory is that the parent going from display: none to visible, triggers the resize of the gridster element before the tab allocates a proper width (height should be gererated by the gridster element). So the gridster will initially render in mobile mode.

Then, the tab will calculate it's proper width and gridster will readjust accordingly, causing the flicker.

You could probably identify if this is the case by logging width's during the resize code.

danomatic commented 9 years ago

The latest branch is attempting to avoid triggering is mobile or any other size changes if the gridster element is display none. However if as parent element is display none, it doesn't handle that. I'm not seeing a flicker when using ng show and hide directly on the element.

mtraynham commented 9 years ago

Yeah, display: none doesn't really propagate to the child nodes in any form. jQuery has the $(element).is(':visible') feature, but this walks the node tree looking for a parent with display: none... I wouldn't say that is particularly performant, and it requires the full version of jQuery. Tabs do have the active bindable property though, and I've used that in the past as a flag when to render things. So maybe that is not a terrible alternative, binding to a property that disables/enables resize updates? Just throwing it out there.

ReinierGielen commented 9 years ago

We haven't worked with angular-gridster for a while, but restarted it now. I the mean while we migrated to angular 1.3

And now my dashboard with ui-bootstrap tabs and a dashboard in each tab is not working properly.

From the discussion above I can not clearly extract a work around or solution.

Is there a way to get this working again ?

griessbreilp commented 9 years ago

I might have a solution for your problem ReinierGielen. My problem was, i had a navigation with Buttons, which i can drag and resize. After i switched the tabs from Bootstrap, the Buttons didn't had their inline CSS anymore. Only if i resized or double clicked somewhere, the inline CSS got reinserted.

So i look what the issue was, and i think that angular gridster has a problem to update the "isMobile" variable and the watch method, according to the tab.

in your Gridster Options set "mobileModeEnabled" to false.

When you switch tabs, the size changes, so you get in the mobile mode, even if the "isMobile" Options is set to false.

I hope that solve your problem.

ReinierGielen commented 9 years ago

Thanks for the advice, but it does not seem to work. However I have found a work around. I placed the dashboards outside the tabs. So basically the tab's are the tab buttons but there is nothing in the content area. Below the tabs I have a ng-repeat that generates all my dashboards. Each dashboard has a ng-show linked to the active tab.

Works like a charm.