Skip to content
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

Quick Editing in Table Editor Widget #12129

Merged
merged 16 commits into from
Jan 30, 2025

Conversation

farmaazon
Copy link
Contributor

@farmaazon farmaazon commented Jan 24, 2025

Pull Request Description

Fixes #10865

The proper "tabbing" cells is implemented in AgGrid, but wasn't visible due to a bug. This PR adds only header edit handling.
"Entering" has our own implementation, as I haven't found a way to configure AgGrid to make behavior described in the task requirements.

Screencast.From.2025-01-29.13-42-00.mp4

Important Notes

The "quick" editing uncovered a problem with our AgGrid updates: the component's refreshing was callled way too often. In this PR it is addressed by reducing reactive dependencies of rowData and columnDefs, so they don't cause updates when only value changes (on value change, we refresh AgGrid by hand: this does not stop editing).

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • [ ] The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • [ ] 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.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

Copy link

github-actions bot commented Jan 24, 2025

🧪 Storybook is successfully deployed!

📊 Dashboard:

@farmaazon farmaazon marked this pull request as draft January 24, 2025 12:13
@farmaazon farmaazon force-pushed the wip/farmaazon/table-editor-quick-edit branch from 7b21c0e to 28353ef Compare January 27, 2025 13:50
@farmaazon farmaazon force-pushed the wip/farmaazon/table-editor-quick-edit branch from 00bf71a to e964474 Compare January 29, 2025 12:15
@farmaazon farmaazon marked this pull request as ready for review January 29, 2025 12:46
Comment on lines 67 to 71
suspend: () => {
return {
resume: () => syncGridWithEditedCell(),
}
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this do anything? A suspend implementation that doesn't capture any state is suspicious.

for (const [key, comp] of Object.entries(props.components)) {
class ComponentWrapper implements IHeaderComp {
private readonly container: HTMLElement = document.createElement('div')
private handle: VueComponentHandle | undefined

init(params: IHeaderParams) {
this.handle = vueHost.register(h(comp, params), this.container)
console.log('init')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray log (and below)

@@ -32,11 +32,11 @@ export const [provideWidgetTree, injectWidgetTree] = createContextStore(
/** TODO: Add docs */
export function useCurrentEdit() {
const currentEditRoot = shallowRef<WidgetEditHandlerRoot>()
return {
return proxyRefs({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is a proxyRefs, we must not destructure it in the call above.

@farmaazon farmaazon added the CI: Ready to merge This PR is eligible for automatic merge label Jan 30, 2025
@mergify mergify bot merged commit 5c07590 into develop Jan 30, 2025
40 of 42 checks passed
@mergify mergify bot deleted the wip/farmaazon/table-editor-quick-edit branch January 30, 2025 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-gui CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Table Editor Widget - Quick edit mode
2 participants