Redesign: new layout algorithm for room sublists. #2507
Conversation
caused by mixing up absolute height with incremental height
| } | ||
|
|
||
| setHeight(height) { | ||
| this._layout._relayout(this._anchor, height - this._initialHeight); |
There was a problem hiding this comment.
hm, i'm surprised that you're throwing away the result of _relayout.
When dragging the handle, the prototype has:
if (Math.sign(offset) != Math.sign(lastOffset)) clampedOffset = undefined;
if (clampedOffset !== undefined) {
if (offset < 0 && offset < clampedOffset) return;
if (offset > 0 && offset > clampedOffset) return;
}
clampedOffset = layout(anchor, offset);
if (clampedOffset !== undefined) {
console.log(`layout is clamped to offset ${clampedOffset}`);
}This handles quite an important edge case where if you just keep recalculating the layout as the user resizes beyond the constraints of the layout, then you can find the layout suddenly popping into other solutions (typically, all the 'above' sections suddenly popping to be minHeight if you attempt to resize a given section to be massive). This is probably only true of the blend algorithm (which may be why you haven't seen it), and i concluded it was a feature of the alg rather than a bug.
So to mitigate it, you need to track whether the layout is already constrained (i.e. returns constrainedOffset != undefined) and is trying to push beyond the constraint, and if so abort the resize. This in turn needs an edge case so that if you change resize direction you mark the layout as not constrained, otherwise it can get stuck in constrained mode.
Given this probably isn't needed when in not-blended mode, it's not a disaster to skip implementing it - am including the explanation here for posterity.
There was a problem hiding this comment.
(n.b. i mentally s/clamped/constrained/g here to match https://github.com/matrix-org/matrix-react-sdk/pull/2507/files#r251215622)
There was a problem hiding this comment.
Right, will do this (and the other feedback), but I got it in a usable state now and @lampholder suggested to get it in front of people ASAP so we know we're headed in the right direction for this. So propose to merge when the build succeeds, and have (immediate) follow-up PRs. Sgty?
|
this looks super promising to me - have made some minor comments to the WIP, but it's pretty much as i'd expect. thank you for grokking and porting the prototype over. can't wait to see if anyone else likes it! :) |
effectively get rid of _originalHeights and calculate the array from the dictionary when needed
element-hq/element-web#8125