angular-ui / ui-layout

This directive allows you to split !
http://angular-ui.github.io/ui-layout/
MIT License
405 stars 194 forks source link

Window resize resets column sizes #100

Open mohsen1 opened 9 years ago

mohsen1 commented 9 years ago

See this Plunker. If you change the size of the panels, resizing window (or the preview pane in this case) resets the adjusted ratio.

You can checkout Swagger Editor to see the same behavior

First reported : https://github.com/swagger-api/swagger-editor/issues/458

alexandre-tobia commented 9 years ago

Any fix for that? same issue here...

astefan92 commented 9 years ago

Same issue, very annoying. Any fix?

ghost commented 9 years ago

Getting the issue here too on chrome

ghost commented 9 years ago

This could be a hard problem to fix.

nitech commented 9 years ago

It looks like it has something to do with https://github.com/angular-ui/ui-layout/blob/298ff5bfb03b41332a6220a223607b434e90ae58/src/ui-layout.js#L154

j--w commented 9 years ago

I'm looking into this one today.

j--w commented 9 years ago

Believe I have a fix for this, need to tidy up and write a test before submitting PR.

nitech commented 9 years ago

Great j--w. I'll be the first to verify that it works :-)

DjRAST commented 9 years ago

Hi, will the fix also resolve the problem that the entire toggle-functionality is corrupted when resizing while a container is collapsed? Thanks!

j--w commented 9 years ago

DjRAST the solution I'm working on does not resolve that issue, though it is related and what I have could be an incremental fix towards resolving the behaviour you're talking about as well.

DjRAST commented 9 years ago

Thanks j--w. I found a workaround for my issue: I expand every collapsed container automatically on resize. It's not perfect but at least no broken layout state ocurs then

j--w commented 9 years ago

I'm going to be on vacation this week and it's doubtful I'll get time to work on this before then. If anyone else is interested in working on it you can have a look at my fix https://github.com/j--w/ui-layout/commit/7167719c77ca8e78f0ef569df50e515474b369c9. It's a bit dirty so if anyone wants to write a test and clean it up a bit go ahead. If this is still unresolved when I'm back from vacation I'll have another crack at it.

nitech commented 9 years ago

I hope your vacation was great ;-) How's #100 doing?

nitech commented 8 years ago

@j--w How's stuff going with #100?

j--w commented 8 years ago

@nitech Haven't looked at it in a few weeks to be honest, but thanks for the reminder. I'll try to have another look at it when I wrap a few other things up hopefully this week.

nitech commented 8 years ago

Thanks @j--w. Keep up the good work :-)

petrsimon commented 8 years ago

Hi, I'm working on #152 (and #128) and I ran into this issue witch turn out to be closely related. @j--w, I've looked at your branch where you implement the cache, but I've got a slightly different idea. Please have a look at https://github.com/petrsimon/ui-layout/tree/refactor-display-update and let me know what you think. Instead of a new cache object, I store the sizes in the containers themselves. The commit looks little messy, due to couple of renames and little bit of formatting. Sorry about that. Branch refactor-display-update solves this issue, but it's not a final solution (there's an issue with initially collapsed containers, which work, but don't toggle properly - I have a solution for that as well in branch https://github.com/petrsimon/ui-layout/tree/toggle-and-resize which is a merge of #152 and this one) Cheers

j--w commented 8 years ago

@petrsimon I think that's a good approach and addresses some of the issues I was having. Have you created a plunkr using this branch so that these changes can easily be demonstrated?

I would argue that the messy history and formatting changes create a lot of noise in the diff that may make this a hard branch to create a PR from. Not sure how @BobbieBarker and @SomeKittens feel about that. I'd love to see this stuff get in so we can close those issues though, let me know if theres anything I can do to help.

petrsimon commented 8 years ago

Thanks, @j--w. You can get b4eb6c2, a new branch that fixes this and is cleaned up for PR. See if anything breaks ;). A plunker for convenience http://plnkr.co/edit/u8hJG42cMQDg9KiG86as.

j--w commented 8 years ago

Looks like the plunker is broken and also some of the tests are failing on your PR. Are you able to have a look at that?

petrsimon commented 8 years ago

Hi, the plunker works fine for me. What errors are you getting? As for the failing test in the PR: it's just a jshint warning about one extra long line. I left that there on purpose, because if I correct it, it will change formatting of the whole file and make reviewing difficult. We can fix that once it's been reviewed.

nitech commented 8 years ago

I'm getting this error:

image

petrsimon commented 8 years ago

I see. That would be on IE? I'm getting too used to es6 or 7 in this case... Can you please try on a different browser for the time being, I'll fix that.

nitech commented 8 years ago

This is on Chrome 46

nitech commented 8 years ago

Getting slightly different message, but on same line, on Firefox:

image

petrsimon commented 8 years ago

Right. It's just in the plunker, so it's easy to change. That line is actually not even used. Done. If you forked the plunker, just uncomment the offending line (16).

nitech commented 8 years ago

Ah... I disabled adblock on this page, and now it works. It turned out that adblock blocked one of the CDN-scripts.

petrsimon commented 8 years ago

I see. I had to use rawgit to link the files, which the adblockers probably don't like. Hope all's good now.

widmoser commented 8 years ago

This seems to work now on the master branch, but there is another issue: #162

tony0918 commented 8 years ago

It looks like doesn't work with size=xx% attribute.