Autosize node based on children #681
Conversation
…t to fall in line with css and renderer 2
src/core/Autosizer.ts
Outdated
| w: number, | ||
| h: number, | ||
| ) => { | ||
| if (carryOver === true) { |
There was a problem hiding this comment.
what does carryOver do/mean?
There was a problem hiding this comment.
forget about this, I removed it and changed a bit of logic to avoid unnessecary calcs.
wouterlucas
left a comment
wouterlucas
left a comment
There was a problem hiding this comment.
nice - looks good, minor remarks.
it would make sense to test this with some large deeply nested child trees. Just to profile what the impact is of having a lot of children/deeply nested children when navigating around.
src/core/Autosizer.ts
Outdated
| } | ||
| } | ||
|
|
||
| patch(node: CoreNode) { |
There was a problem hiding this comment.
would it be cheaper to go by ID only?
basically:
patch(id: number) {
since you're getting the entry anyway from the this.childMap, saves pushing a big node reference across.
Also entry isn't used further down - it would make that actually be used 😄
src/core/Autosizer.ts
Outdated
| if (corner.y > maxY) maxY = corner.y; | ||
| } | ||
| } | ||
| this.flaggedChildren.length = 0; |
There was a problem hiding this comment.
this does mean the array can be adjusted while we are in the for loop, correct?
would it be better to copy the contents and then clear the array, to avoid race conditions for the price of a little extra memory usage?
src/core/CoreNode.ts
Outdated
| this.isRenderable === true && | ||
| this.parentAutosizer !== null | ||
| ) { | ||
| this.parentAutosizer.patch(this); |
There was a problem hiding this comment.
could do patch(this._id)
| } | ||
| } | ||
|
|
||
| detach(node: CoreNode) { |
There was a problem hiding this comment.
this could also be by id instead of CoreNode to reduce GC pressure a bit
There was a problem hiding this comment.
agreed, on later review, this avoids extra map call.
Add autosize to a node to dynamically update its size based on children's matrix3d.
autosizepropertynotes:
PR includes unit tests and VRT testcase.
Useful for Touch/Point usecases where you want the parent to be clickable and may provide utility for flexbox usage.