josdejong / svelte-jsoneditor

A web-based tool to view, edit, format, repair, query, transform, and validate JSON
https://jsoneditoronline.org
Other
814 stars 108 forks source link

feat: hide context menu when onRenderContextMenu return false #411

Closed 1pone closed 3 months ago

1pone commented 3 months ago

This is an optimized version of pr https://github.com/josdejong/svelte-jsoneditor/pull/399

josdejong commented 3 months ago

Thanks! I think this is exactly the logic we need.

I have a few feedbacks regarding the code:

  1. I think it would be nice to keep the logic to create the menu items in a separate file, since the files TreeMode.svelte and TableMode.svelte are already very large. We can create a function like const items = createTreeContextMenuItems({ json, documentState, parser, ..., onCut, onCopy, onPaste, ... }) and put that in a separate file. Also, the components TreeContextMenu and TableContextMenu are now about empty and don't add anything anymore, I think we can remove them. So basically: we can replace the component TreeContextMenu.svelte with a function in a file createTreeContextMenuItems.ts, mostly containing the same logic as the component had (and do the same with table mode). What do you think?
  2. Minor thingy: can you replace the inline return like if (items === false) return to have braces, like:

    if (items === false) {
      return
    }

    (a personal preference)

1pone commented 3 months ago

Thanks, I've made the changes you suggested.

1pone commented 3 months ago

Resolved

josdejong commented 3 months ago

Thanks!

I was thinking: this still has to be published as a breaking changes: when you had an onRenderContextMenu before and was returning something, you now have to handle the case when the editor is readOnly.

1pone commented 3 months ago

That's right, just like the adjustments made in the development page, this should really be known by those who are or have used onRenderContextMenu.

josdejong commented 3 months ago

I had one more thought: now that the context menu can be visible when the editor is readOnly, all the buttons should reckon with it and be disabled when not applicable in read-only mode. I've worked that out in c66ee09. I also came a cross a few minor issues related to the context menu, fixing that too.

Also, I added a property readOnly to the context of the onRenderMenu and onRenderContextMenu callbacks, see 4df55481a17c22211538df1cd289faec3af25177.