Skip to content

fix(data_editor): return true from onDelete so Delete clears the selection - #7884

Open
VishApp wants to merge 5 commits into
reflex-dev:mainfrom
VishApp:dataeditor-on-delete-return-true
Open

VishApp wants to merge 5 commits into
reflex-dev:mainfrom
VishApp:dataeditor-on-delete-return-true

Conversation

@VishApp

@VishApp VishApp commented Nov 13, 2026 •

Copy link
Copy Markdown

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Description

Closes #7852

Glide calls onDelete synchronously and uses its return value: true deletes the selection, false cancels, and anything else is treated as the selection to delete. Reflex rendered on_delete as a normal event callback, which returned the result of addEvents (undefined). Glide then read that as a selection and threw TypeError: toDelete.rows is not iterable, so the cell kept its value even though the handler received the selection.

  • DataEditor._render now wraps onDelete so it queues the Reflex event and then returns true, letting Glide delete the selection as it does when on_delete is not bound.
  • The wrap happens at render time, after event-trigger memoization, so the useCallback dependencies of the handler are unchanged. The cost is one small arrow function per render of the grid.
  • A backend handler can't cancel the delete or pick a different selection, since it can't answer Glide synchronously. The issue lists that as out of scope.
  • I checked the other Glide callbacks Reflex exposes as event handlers: onDelete is the only one whose return value Glide uses synchronously (onRowAppended accepts undefined).

Tests

  • Unit test (tests/units/components/datadisplay/test_dataeditor.py): runs the rendered onDelete under node and asserts it returns true and queues exactly the expected event with the selection. Fails on main.
  • Playwright test (tests/integration/tests_playwright/test_data_editor.py): uses the app from the issue, clicks the second row, presses Delete, and asserts the handler got the selection, the cell was cleared through on_cell_edited, and no page error occurred. Passes in dev and prod; fails on main with the TypeError.
  • uv run pytest tests/units --cov: 15223 passed, coverage 81.68%.
  • ruff check, ruff format, pyright reflex tests and the pre-commit hooks pass. The biome hook could not install locally (npm git-dependency restriction); this PR has no JS/JSON changes.
  • News fragment added in packages/reflex-components-dataeditor/news/.

AI-assisted: I used Claude Code to help write this change and reviewed and tested it myself.

VishApp and others added 4 commits November 13, 2026 13:53
Glide reads the return value of onDelete and treats any non-boolean as the selection to delete. The generated callback returned the result of addEvents, so pressing Delete with on_delete bound threw a TypeError and left the cell unchanged. Fixes reflex-dev#7852.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@VishApp
VishApp requested a review from a team as a code owner November 13, 2026 08:25
@greptile-apps

greptile-apps Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] This PR appears safe to merge.

Summary

This PR fixes Delete in data editors with on_delete bound while keeping the backend notification.

  • Delete clears selected cells when a data editor has an on_delete handler.

No actionable issues found.

Reviews (2) · Last reviewed commit: "Merge branch 'main' into dataeditor-on-d..." · Reviewed by Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 4 files

View guided diff | Re-trigger cubic

@VishApp VishApp changed the title Dataeditor on delete return true Let rx.data_editor clear cells on Delete when on_delete is bound Nov 13, 2026
@VishApp VishApp changed the title Let rx.data_editor clear cells on Delete when on_delete is bound fix(data_editor): return true from onDelete so Delete clears the selection Nov 13, 2026
@FarhanAliRaza FarhanAliRaza added the bug Something isn't working label Nov 13, 2026 — with ChatGPT Codex Connector

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rx.data_editor with on_delete bound: pressing Delete throws TypeError: ... reading 'length' and never clears the cell

2 participants