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

Resizing a grid window shudders down the page with pinned columns #1957

Closed
morungos opened this issue Oct 29, 2014 · 9 comments · Fixed by #3371
Closed

Resizing a grid window shudders down the page with pinned columns #1957

morungos opened this issue Oct 29, 2014 · 9 comments · Fixed by #3371
Assignees
Milestone

Comments

@morungos
Copy link
Contributor

When there's a pinned column and the grid has width 100%, the non-pinned part flickers between the alongside and below the pinned part. In some cases, it even sticks in the broken position, where the two viewports remain disconnected.

To reproduce, see the jsfiddle: http://jsfiddle.net/14mLg0aw/embedded/result/. Make the window smaller to see the effect. Key factors issues are the "width: 100%" CSS and the use of pinned columns. Sadly the full width is a requirement, but workarounds to achieve this effect would be welcome.

This is the same on Chrome, Safari, and Firefox (all tested on OSX).

It's possible that this is related to #1426, but I don't think so (unlike that issue, this also happens on Mac Firefox).

@morungos
Copy link
Contributor Author

Well, this is my first delve into ui-grid code, but the issue seems to be in calls to Grid.prototype.registerStyleComputation, at least in part. The width calculation updates are being registered multiple times, probably because there are two uiGridRenderContainer elements within the grid. This is why it only happens when pinned elements are present, as otherwise there's only one.

These are interleaved with scroll bar width calculations, which are generating recalculations both with and without scrollbar sizes. When shrinking a window, this means alternating between OK and too small, causing the floats to wrap.

There are probably several things wrong here that I don't get, but certainly the multiple updates is going to be an issue.

@PaulL1
Copy link
Contributor

PaulL1 commented Nov 4, 2014

Yeah, we've had ongoing issues with this, in some cases it always rendered wrong. I thought we'd addressed most of it, the problems we had in the past were largely down to off-by-one and rounding errors, which resulted in the viewport being slightly too wide, and therefore the two renderContainers stacking.

It is a complex area, and fiddly to work on. The width calculations are presumably being registered multiple times because they register in each of the render containers? (i.e. they're not registered twice in the same container?).

@PaulL1 PaulL1 added this to the 3.0 milestone Nov 4, 2014
@morungos
Copy link
Contributor Author

On looking a little more deeply, I don't think this is off-by-one, but is basically down to use of float: left for the components. Shrinking a window composed of floats which have fixed sizes causes a reflow, which then gets fixed a bit later by the queued recalculation. Although the calculation might happen several times, Angular's generally only picking up what's left at the end, so the inefficiency isn't there: it's in the CSS reflows. I've quickly tried fakery to add containers which have overflow-x: hidden but so far without success. I think I'll need to build a small environment containing the big pieces, header, footer, pinned views, scrollbars, and just see how to assemble them into something that vaguely works, probably without any of the ui-grid CSS magic getting in the way. It doesn't help that I don't have Windows or Linux systems to test on, but that's a problem I can probably resolve at my end.

I keep feeling the dynamic CSS changing after initialization really really should not be required, and if something is, it ought to be much simpler. I'm not a huge CSS expert, but it feels ui-grid is trying to do too much of the heavy lifting to me. Putting in column sizes in CSS feels nice, but dynamically changing viewport sizes that way is, I'm beginning to feel, inviting trouble we don't need.

OSX doesn't help with its magic scrollbar widths, which genuinely can change from 15 to 0 while the browser is still open (if, say, a mouse is unplugged and a display changed).

Just thoughts at this stage, but reciprocal/contrary opinions and ideas very welcome. I have effort to put into this, any pointers as to where to go with it very welcome.

@PaulL1
Copy link
Contributor

PaulL1 commented Jan 14, 2015

I think this is probably down to digest cycles, and the browser resizing before ui-grid gets time to change column widths. I'm not sure I have a great answer as to how to resolve it.

@PaulL1
Copy link
Contributor

PaulL1 commented Jan 16, 2015

OK, worked through this some more. The issue looks to be that we're using queueRefresh() in the resize. This means that we complete the digest cycle, then resize the grid in a timeout afterwards. I surmise that this gives the browser time to draw the grid incorrectly, then in the timeout we fix it, causing the juddering.

I've fixed by changing to a grid.refreshCanvas immediately, rather than queueing the refresh. This makes sense to me because there should be nothing else running in a resize digest cycle, so there's no reason there would be a performance benefit from debouncing the refresh calls when handling this event.

@PaulL1
Copy link
Contributor

PaulL1 commented Jan 17, 2015

Looks like this isn't resolved - it worked on my test case, but not on the plunker (once the code was merged).

@PaulL1 PaulL1 closed this as completed in e839bf5 Jan 21, 2015
PaulL1 added a commit that referenced this issue Jan 21, 2015
Fix #1957 (edit): cancel all eventHandlers
@c0bra c0bra removed the in progress label Jan 21, 2015
@PaulL1 PaulL1 reopened this Jan 21, 2015
@PaulL1
Copy link
Contributor

PaulL1 commented Jan 21, 2015

Wrong issue. :-(

@PaulL1
Copy link
Contributor

PaulL1 commented Mar 28, 2015

Same jsfiddle but in a plunker so that I can include a new ui-grid version. I've made changes, which I had hoped would fix things. They seem to make it a bit better, but I think we're still fundamentally stuck with needing a digest cycle before the grid resizes itself to the available space, and in the meantime the browser renders the sizing that it already had.

http://plnkr.co/edit/sm8AuildYJVl9Kk6rcek?p=preview

PaulL1 added a commit to PaulL1/ng-grid that referenced this issue Mar 28, 2015
Change the style computations with the aim of making them deterministic
and able to calculate widths in a single pass rather than being iterative.

Fix angular-ui#3050, and somewhat improves angular-ui#1957, at least to my eye.

Changes are:
- change renderContainer updateColumnWidths to calculate across all
  visible columsn in all render containers, rather than trying to calculate
  one render container at a time (cannot otherwise work with * and %, these are of
  the whole grid, not just one render container)
- change header updateColumnWidths to just rely on the renderContainer, not
  do it again itself in a slightly different way
- change updateColumnWidths to be simpler, and to calculate percentages in a way
  that I think makes more sense
- change resize feature to not attempt to 'resizeAround' the changed column, but
  rather allow updateColumnWidths to deal with it
- change pinning to not call refresh twice when moving a column, this is now
  unnecessary
- updates to tutorials
@c0bra c0bra assigned c0bra and unassigned PaulL1 Apr 16, 2015
c0bra added a commit that referenced this issue Apr 27, 2015
Pinned containers were wrapping in certain scenarios where grids were being resi
zed. It looked to the end-user like a poorly-rendered flash, and could possibly
happen over and over during resizing.

This fix uses margins to offset the body render container from the the left and
right render containers

Fixes #2997, Fixes #1957
@sebilozano
Copy link

Was this ever resolved? I have different grids in tabs and am currently using 'ui-grid-auto-resize' to resize when shown. Each of these grids has a column pinned on the right and sometimes when I load the grids, the pinned column begins to flicker. Any ideas?

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

Successfully merging a pull request may close this issue.

4 participants