-
Notifications
You must be signed in to change notification settings - Fork 315
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
ElementDocument::UpdateLayout Resets Scrollbars #452
Comments
Additionally, im observing some odd behaviour with having my scroll container inside a padded flex box container. Do we have some sort of infrastructure for a minimal test example? like some Rml-Player to quickly share and evaluate documents? That would be neat for situations like this |
I found the real issue, ElementDocument::UpdateLayout resets scrollbar positions in unrelated elements. Building a test case now |
Here is a minimal example, run it for instance using the player i made in #453 . The issue only seems to occur with scroll containers inside of flex boxes. Additionally the error is different depending on if the list is in a tabset or not. <rml>
<head>
<style>
* {
box-sizing: border-box;
}
body {
font-family: LatoLatin;
}
scrollbarvertical {
width: 15px;
}
scrollbarvertical sliderbar {
width: 15px;
background-color: white;
}
.outer {
display: flex;
width: 100vw;
height: 200px;
}
.container {
width: 50%;
display: flex;
flex-direction: column;
padding: 15px;
border: 1px white;
}
.list {
display: block;
height: 100%;
overflow-y: auto;
}
.list>div {
background-color: blue;
display: block;
}
p {
line-height: 40px;
}
button {
display: block;
padding: 10px;
background-color: red;
}
button:hover {
padding: 20px;
}
</style>
</head>
<body>
<div class="outer">
<div class="container">
<tabset>
<tab>tab</tab>
<panel>
<div class="list">
<div>
<p>elem1</p>
</div>
<div>
<p>elem2</p>
</div>
<div>
<p>elem3</p>
</div>
<div>
<p>elem4</p>
</div>
<div>
<p>elem5</p>
</div>
<div>
<p>elem6</p>
</div>
</div>
</panel>
</tabset>
</div>
<div class="container">
<div class="list">
<div>
<p>elem1</p>
</div>
<div>
<p>elem2</p>
</div>
<div>
<p>elem3</p>
</div>
<div>
<p>elem4</p>
</div>
<div>
<p>elem5</p>
</div>
<div>
<p>elem6</p>
</div>
</div>
</div>
</div>
<button>hover me</button>
</body>
</rml> |
Thanks for the detailed report, I can easily reproduce the issue with it. This one might be quite tricky, I'll take a closer look at it at some point. |
Co-authored-by: Dakror <mail@dakror.de>
I have run into the same issue. I just want to highlight that it indeed seems to happen only within a flexbox container which is an incredibly great find. Working around that and working with CSS alternatives (temporarily) fixed the scrollbar being reset to 0. |
I want to add to this that this occurs whether or not any parent is a flex box, not just the immediate parent. I have a layout heavily dependent on flex positioning and this is causing a lot of issues for me and can't find a workaround without a hefty redesign. Has any progress been made on this issue? |
I tried looking into this issue by adding an if (GetScrollHeight() != GetClientHeight())
{
scroll_offset.y = Math::Min(scroll_offset.y, GetScrollHeight() - GetClientHeight());
} So it may be that |
Thanks for reporting in. I haven't actually been able to prioritize this one yet unfortunately. I think I have a vague understanding of it, but I'm not sure exactly how to approach it. It might need some architectural improvements. My theory of what is happening, is that the layout engine always starts out with the assumption that scrollbars are not necessary. Then, once it starts doing layouting, it is updating sizes based on that assumption. In some cases like those reported here, it is effectively zeroing out the scroll offset since the sizes are too small to allow scrolling. Then, once it later knows it needs scrollbars, things are layed out again, this time ending up in the right sizes. However, since the scroll offset was already zeroed out in the previous step, it can no longer get back to the original offset. Indeed, as you discovered, we can effectively work around this by removing the scroll offset clamping in the |
Yeah, that makes sense. One thought I had is that it could make sense to cache the scroll position until updates are complete, so that it can be reset on the final update. I don't fully understand the update loop so you'd probably have a better idea than myself what's best here. |
I am pretty sure this will cause other issues, so we should look for another solution. I did have one idea though, to simply wait with clamping until all the layouting is one, and then clamp at the end. I implemented it a bit quickly here, do you want to test this one: 5a2d1cf I am not fully sure about all the implications, but it could be a possible fix, and it is quite simple too. |
Just tested it! That works perfectly, no issues on my end in any of the scroll areas that were having this issue before. I have an area that has dynamic elements which get swapped with a toggle, and the scroll position was retained even though every child completely changed, so this seems very robust! |
Great to hear! I want to test it a bit more before I merge to master, but that seems promising. |
I have a max-height constrained container inside a tabset panel and scroll progress is always reset to 0 after one context update.
I tested this even with a completely unstyled naked tabset. When commenting out the tabset, scrolling works normally.
The text was updated successfully, but these errors were encountered: