panes
panes copied to clipboard
[BUG] calcFitHeight calculates wrong value
Describe the bug When using the fitHeight option, changing the drawer content from 200px to 100px and call calcFitHeight(), the content grows instead of shrinking.
To Reproduce https://jsfiddle.net/jwe3r4uy/
Expected behavior When changing the content from 200px to 100px, the drawer should shrink.
Do you think there is any workaround till its fixed? When I see it correctly, its caused by an interal variable that holds the previous value and is added as 'diff'. So currently I dont see a possible workaround, but maybe you have an idea.
@AE1NS ill check it in few day, thank you for issue
@AE1NS Please check version from master branch. If that's fixed i gonna push the tag version up.
Just tested it. Looks good, thank you!
Unfortunately there is another issue now. When you change the content height to more than 100% of the pane and change it back, the calculated height is wrong (in the example it just moves out of the window at the bottom). Here is an example: https://jsfiddle.net/0oy71j2c/
@AE1NS big thank you for your support. Should be fixed and pushed v1.3.13
.
FitHeight logic quite fresh and I wouldn't able to test all cases, do not hesitate to create new tickers if so.
Maybe it is a good point to create automated screenshot testing with cypress, to reduce such cases.
Unfortunately there seems still to be issues with calcFitHeight.
Could you have a look to this example: https://jsfiddle.net/nzb4gphe/
At the end, the container does not have the correct height, its larger.
@AE1NS Thank you for report. Does it happens after recent tag release v.1.3.31
?
I dont know exactly. We used 1.3.21 and I had problems with wrong pane heights. I had situations, where the pane sometimes had a wrong height and calcFitHeight didnt solve it. Thats why I upgraded to latest and created the jsfiddle based on it. But I dont know if its the same cause of the problem. When I had the problem in our app, I had to remove the height values of the wrapper container and then calcFitHeight fixed it, but with ugly animations because some bottom shaddow of a wrapper container was visible while animation.
@AE1NS Ok I'll re-check.
I just see thats its even more problematic on 1.3.31 than on 1.3.21. With 1.3.31 I have more cases where the pane is not calculated correctly. In my case the pane gets not shrinked and stays at the full previous height.
@AE1NS Please check your example and also https://jsfiddle.net/aenfx4gc/ Looks like all cases are covered properly.
Thanks for the update! Could you have a look at my jsfiddle example again. Its technically working now, but shouldnt be the pane still be 100% height when the animation starts (as it worked before)? Now the container 'jumps' small to 100px and animates down afterwards. Thats the same effect as in the previous version, when I just removed the 'height' property and called calcFitHeight and looks really strange as I also noted in my last comment (with the bottom shadow).
@AE1NS You are absolutely right. I will improve transitions to be a smooth.
@AE1NS Please check.
There are still problems, the button is pushed out at the bottom here: https://jsfiddle.net/ezmct1L5/
The height should also be handled correctly, when the rule * { box-sizing: border-box; }
is set for the application, because the wrapper will change from 115px to 100px then and the child stays at 100px.
@AE1NS Checked all your cases, reviewed algorithm and seems like works now for all. Thanks for your big help on improvements. Please check.
There seems to be another strange behavior: https://jsfiddle.net/8dy0wjt2/ The last container has a wrong height. If calcFitHeight is called after another second its correctly, buts it seems like another bug no?
In my use case in my app, I also have two steps/times I call calcFitHeight, when the content has the same size. I just see that the pane element sets a 'height' css value and doesnt remove it. In the other cases when the height changes, it always gets removed automatically.
@AE1NS Thank you again. Fixed also. If you wish, you can make a pull request for some more bugs you find in future. And I will complete pull with fixes, so you can be in contributors.
Looks good for the moment :)
I really appreciate your work and this awesome support! Thank you very much!
I'm using latest version, 1.3.51, and I just can't call this function. It says '.calcFitHeight is not a function'. I'm using overflow-y to set the scrollable area, but everytime I navigate it breaks, it jjust doesn't calculate the height, therefore no scroll. I wanted to trigger it manually if it's the case, but I can't call this function u guys were talking about. @roman-rr @AE1NS
@luckashenri please create new issue and show your code. I just checked and it works on my side. https://jsfiddle.net/romantonoff/sq2va3xc/