Skip to content

#208 Call onEditEvent when starting and stopping "add key" UI #210

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

Merged
merged 9 commits into from
Jul 10, 2025

Conversation

CarlosNZ
Copy link
Owner

Fix #208

Note, does not call event when the actual key is added to the data structure, only when instantiating the "new key" UI (and stopping it).

Copilot

This comment was marked as outdated.

@CarlosNZ CarlosNZ requested a review from Copilot July 10, 2025 12:14
Copilot

This comment was marked as outdated.

@CarlosNZ CarlosNZ requested a review from Copilot July 10, 2025 12:21
Copilot

This comment was marked as outdated.

@CarlosNZ CarlosNZ requested a review from Copilot July 10, 2025 12:25
Copilot

This comment was marked as outdated.

@CarlosNZ CarlosNZ requested a review from Copilot July 10, 2025 12:31
Copilot

This comment was marked as outdated.

@CarlosNZ CarlosNZ requested a review from Copilot July 10, 2025 12:35
Copilot

This comment was marked as outdated.

@CarlosNZ CarlosNZ requested a review from Copilot July 10, 2025 12:41
Copilot

This comment was marked as outdated.

@CarlosNZ CarlosNZ requested a review from Copilot July 10, 2025 12:43
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures the onEditEvent callback is fired when the “Add key” UI starts and stops, without firing when the actual key is added to the data.

  • Extends the OnEditEventFunction signature to allow null entries in the path array and makes onEditEvent optional on relevant props.
  • Invokes onEditEvent in the tree provider and button panel when entering/exiting “add key” mode.
  • Propagates the new prop through JsonEditor, CollectionNode, and updates documentation and demo.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/types.ts Updated OnEditEventFunction signature; added optional prop
src/contexts/TreeStateProvider.tsx Added guarded call to onEditEvent when toggling edit state
src/JsonEditor.tsx Forwarded onEditEvent prop into the editor component
src/CollectionNode.tsx Propagated onEditEvent to the JSON node renderer
src/ButtonPanels.tsx Hooked onEditEvent into updateAddingState for add-key UI
demo/src/App.tsx Added commented example of onEditEvent usage in demo app
README.md Updated callback signature and added usage note
Comments suppressed due to low confidence (3)

src/types.ts:181

  • The updated OnEditEventFunction signature removes support for string paths, potentially breaking existing consumers. Consider supporting both string and array path types or documenting this breaking change in the migration guide.
export type OnEditEventFunction = (path: (CollectionKey | null)[] | null, isKey: boolean) => void

src/ButtonPanels.tsx:81

  • There are no tests covering the new onEditEvent calls when starting/stopping the "add key" UI. Consider adding unit/integration tests to verify onEditEvent is invoked with correct parameters.
    if (onEditEvent) onEditEvent(active ? [...path, null] : null, active)

src/contexts/TreeStateProvider.tsx:93

  • The conditional check for path only allows arrays or null, but if string paths should also trigger the event (as previously supported), they will be skipped. Verify if string paths need to be included here or update the logic accordingly.
    if (onEditEvent && (Array.isArray(path) || path === null))

@CarlosNZ CarlosNZ merged commit fa28ea4 into main Jul 10, 2025
@CarlosNZ CarlosNZ deleted the 208-new-key-edit-event-(simple) branch July 10, 2025 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

onEditEvent is not triggered when adding a new key
1 participant