-
Notifications
You must be signed in to change notification settings - Fork 327
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
Fix Table Input Resizing #12643
Fix Table Input Resizing #12643
Conversation
🧪 Storybook is successfully deployed!📊 Dashboard:
|
…/fix-table-input-resizing
if (!initialBounds) break | ||
bounds.value = initialBounds.withBoundsClamped( | ||
selectFields(resizing, { | ||
top: initialBounds.top + pos.relative.y, | ||
bottom: initialBounds.bottom + pos.relative.y, | ||
left: initialBounds.left + pos.relative.x, | ||
right: initialBounds.right + pos.relative.x, | ||
}), | ||
) | ||
else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be simplified to if (initialBounds)
const scale = computed(() => navigator?.scale ?? 1) | ||
provideResizableWidgetRegistry( | ||
computed({ | ||
get: () => visualizationWidth.value && visualizationWidth.value * scale.value, | ||
set: (width) => (visualizationWidth.value = width && width / scale.value), | ||
}), | ||
() => CONTENT_PADDING * scale.value, | ||
() => widgetTreeSize.value.x, | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may cause a performance problem in large graphs--adding a reactive dependency on navigator.scale
will require Vue to update every GraphNode
every frame while zooming the graph. Wouldn't it be more efficient for the registry to use scene coordinates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GraphNode components should not be updated; nodeWidth
change in resizableWidgetRegistry will trigger a watch calling adjustToNodeWidth
, but here we would just compute exactly the same widget size, which is discarded before writing to metadata.
And because I rely on useResizeObserver
, the code would be launched anyway on zoom in/out. The optimized solution would require a completely different solution. I assume what is so far is enough - but I test it with 100 nodes yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it on 100 Table.input nodes. It's laggy, but same way as on develop. Interestingly, AgGrid is refreshed there when zooming - probably worth checking at some point.
…/fix-table-input-resizing
Pull Request Description
Fixes #12520
Screencast.From.2025-03-26.11-47-19.mp4
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
[ ] If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,or the Snowflake database integration, a run of the Extra Tests has been scheduled.