josdejong / svelte-jsoneditor

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

Feature-Request: onSelect #163

Closed sruehl closed 1 year ago

sruehl commented 1 year ago

add a onSelect(path: Path, value: any): void similar to onClassName(path: Path, value: any): string | undefined.

Usecase: I have two editors open and when I select one element in the one editor I want to evaluate the path and value and use it to jump to another element in the second editor.

sruehl commented 1 year ago

as a small addon to that: this feature is used already internally for the breadcrumb navigation (just not exposed).

josdejong commented 1 year ago

Yes, good idea! Thanks for your suggestion Sebastian.

Two thoughts in this regard:

  1. I think an onSelect callback will mostly be useful if there is a corresponding method like .select(...) and/or option { selection: ... } to change the selection too.
  2. We'll need to cater for three different types of selection: in tree mode , text mode, and table mode. Or we need to come up with a unified way to describe selections in each mode.
sagIoTPower commented 1 year ago

The pull request #229 add updateSelection to JSONEditor addresses the feature to change the selection. I would be happy if it could be merged into main.

Regards Christof

josdejong commented 1 year ago

Thanks @sagIoTPower. I would like to first think through https://github.com/josdejong/svelte-jsoneditor/issues/163#issuecomment-1277397051 before merging your PR, we may want to go a different direction to cater for the three different modes. Do you have thoughts on a good approach to that?

sagIoTPower commented 1 year ago

Before thinking about a concept for a harmonised approach, I have some observations/ questions:

  1. What is idea behind a unified way to address selections in all modes? In other features the different modes diverge as well. I see scrollTo(path: JSONPath) is implemented in TreeMode and TableMode but not in TextMode.
  2. Although scrollTo(path: JSONPath) is implemented in TableMode but it is never called: snippet from src/lib/components/modes/JSONEditorRoot.svelte
export function scrollTo(path: JSONPath): void {
    if (refTreeMode) {
      return refTreeMode.scrollTo(path)
    } else {
      // TODO: implement scrollTo for text mode

      throw new Error(`Method scrollTo is not available in mode "${mode}"`)
    }
}
  1. If we want to harmonise the selection in all three modes, it would mean to harmonise: function setSelection(anchor, head) -> TextMode and function updateSelection(...) -> TreeMode, TableMode.

Is my understanding correct?

josdejong commented 1 year ago

Thanks for your input @sagIoTPower.

Unified solution: from an end-user point of view it would be ideal to not have to worry different modes, and just having "the" content and "the" selection. However, I think a unified selection is not practically possible because the text mode has a free-text based selection, and tree mode has a JSON based selection. These are just too far apart.

There are indeed more methods that are not supported for all modes, like scrollTo (though this one can be implemented for code mode too). I try to minimize these divergences but we cannot totally avoid it.

Selection API

I think we can go two ways: either create different methods for the different modes, like setTreeSelection, setCodeSelection, etc. But I think that will result in an exploding amount of methods and will be confusing to understand and use. Therefore I think we should come up with a data model that holds the information about both mode and selection. Something like:

interface Selection {
  type: SelectionType, // text or tree, and possibly in the future we need a table specific model
  selection: TreeSelection
}

new JSONEditor({
  target: ...,
  props: {
    // initial state
    mode,
    content,
    selection,

    // listener for selection changes
    onSelect: (selection) => {
      // ...
    } 
  }
})

// a method to change the selection
editor.setSelection(selection)

Refactoring the Selection model

I think we should also refactor the current data model for Selection a bit. Currently, it contains derived information (a pointersMap) derived from the selection and the JSON data, and sometimes has some more redundant information (like anchorPath and focusPath for keys and values, which is always the same). That was there to keep the model unified.

API for the full State

On more thought: instead of separate properties, listeners and methods for mode, content, and selection, we could come up with a single setState and onStateChange, which allows you to get/set the full internal state. To start with, it can hold { mode, content, selection}, and there are a few more internal state variables that we could expose (such as search text). I think though that we should do one step at a time and focus on just the selection now. It may be good to keep this in mind though.

EDIT: also, the expanded state would be useful to control.

howardjones commented 1 year ago

Having spent a while reading the jsoneditor docs and trying to use them with svelte-jsoneditor by accident, I've realised that the onEvent event handler no longer exists - I was hoping to add a context-sensitive help (pulled from jsonschema) alongside my editor as users click through the document... is this intended to replace that?

josdejong commented 1 year ago

@howardjones can you please open a new issue to discuss this? I want to keep this issue focused about onSelect.

howardjones commented 1 year ago

So onSelect is not an event fired when a new field is focused?

josdejong commented 1 year ago

The onSelect event will fire when the selection changes.

I would like to elaborate but let's please discuss it in a separate issue.

josdejong commented 1 year ago

FYI: I'm working on refactoring the Selection type definitions to make them suitable for a public API (right now they contain cached data and are more complex than needed). After that step it should be relatively straightforward to make a public Selection API.

josdejong commented 1 year ago

A new property selection (supporting two-way binding in Svelte), a method .select(newSelection) and a callback onSelect(newSelection) have been implemented in version v0.17.9 now. Please give it a try 😄

sagIoTPower commented 1 year ago

@josdejong, thank you for implementing this feature. :clap: I tested both:

  1. .select(newSelection)
  2. callback onSelect(newSelection)

and it works for my scenario.

josdejong commented 1 year ago

Thanks, good to hear 👌