volto icon indicating copy to clipboard operation
volto copied to clipboard

Query widget paths widgets

Open giuliaghisini opened this issue 2 years ago • 4 comments

Implemented 'paths' widgets for QueryString widget as in Plone 5.

https://user-images.githubusercontent.com/51911425/167152711-28e288fa-53d0-4e01-872a-9e1d1017aa50.mov

Only 'Absolute path' and 'Relative path' options make sense.. 'Navigation path' it is superfluous as it is included in the 'Absolute path' option, so we need to remove this option from plone.rest.api response in @@querystring.

giuliaghisini avatar May 06 '22 14:05 giuliaghisini

Deploy Preview for volto canceled.

Name Link
Latest commit b9cca89476457c84e0eb503b52a6f810312b5e87
Latest deploy log https://app.netlify.com/sites/volto/deploys/656dc84bf66c4800087b9e5d

netlify[bot] avatar May 06 '22 14:05 netlify[bot]

This pr kinda works, but we need to talk about it.

Are we ok to remove somehow the absolute path option? If yes, in plone.volto (it's just a p.a.registry configuration)?

Second: to keep persistence we should store uids instead of paths. We have two ways:

  1. "the classic one" aka "how classic Plone does": in absoultePath query we can store paths or uids and plone.app.querystring can handle both. For example:
{
  'i': 'path',
  'o': 'plone.app.querystring.operation.string.absolutePath',
  'v': '9240f0de502e44b1934fb218f2fc962a::-1'
}
  1. We create a new querystring operation (for example "plone.app.querystring.operation.string.objectBrowserReference") that allows to store a more complex object (with UID, title, etc) and then generates the right query for the catalog.

With first approach, we don't have to touch Plone, but the widget need to do an additional api call when drawing an already populated query because it need to get the Title/path of that UID (Collections widget does exactly that). Also, queriest stored into blocks are more similar to the Collection ones. Cons: we're using an operator called "absolutePath" but we're storing UIDs. And widget need to be customized to make additional api calls.

With the second one, the widget should work as-is but we're storing more infos into query fields and the POST to querystring-search endpoint will be uglier because it will be something like this:

{
  'i': 'path',
  'o': 'plone.app.querystring.operation.string.absolutePath',
  'v': {
    'UID': 123456789,
    "Title": "some title",
    "path": "/foo/bar"
  }
}

As always i can't say if it's better to adapt Volto to Plone or vice versa

@plone/volto-team what do you think?

cekk avatar May 19 '22 14:05 cekk

@plone/volto-team let's talk about it next Volto Team Meeting.

sneridagh avatar May 22 '22 09:05 sneridagh

After discussion in the Volto Team Meeting, we arrived to this conclusions:

We must ensure maximum current "classic" compatibility, so we cannot make any breaking change in the backend right now, meaning we can't change the behavior of the p.a.querystring handling.

However, we could add things, meaning adding the new p.a.querystring operation (which will be used only by Volto), and eventually even by classic.

Another thing is that we are parsing what's coming from the @querystring endpoint, and we might also want to amend that for the Volto use case, or if it's more suitable, deal with the special Volto differences directly in the QueryStringWidget component.

There's also a small UX improvement that it's been brought up: it's the "Navigation Path" is quite confusing and no user knows the difference between that and the "Absolute Path". So we must stick with one. Maybe it's time for Volto to get rid of it, taken into account the first premises I wrote about.

As @cekk pointed out, another option is to ship this "as is" so same features than classic has, assuming the extra call.

sneridagh avatar Aug 03 '22 10:08 sneridagh

@giuliaghisini What is the state here? The missing path widget is still one of the mayor UX-fails in current Volto and I'd love to see this soon.

pbauer avatar Oct 27 '23 10:10 pbauer

@giuliaghisini What is the state here? The missing path widget is still one of the mayor UX-fails in current Volto and I'd love to see this soon.

we left this work on hold because I was absent for a long period, but I confirm that we too absolutely need it. We have to take it back and work on it. Would you like to work on this?

giuliaghisini avatar Oct 30 '23 09:10 giuliaghisini

Update: we're revamping this pull-request with a conservative approach.

We don't need an additional operator from the backend (will close plone.volto related pr)..let's use absolutePath. The widget will do an additional query to @querystring-search with that criteria and depth:0 to get the selected object infos (title). On save, will be stored the uid in "v" because it's way better than the path (basically when you move contents).

With this implementation we don't need an upgrade-step for old stored values because the backend (@querystring-search) can handle both values (path or uid). When we edit the path into a block that previously had an old value (path) and select a new object, the widget will store a uid.

That's it. Any thoughts about that?

Stay connected for more updates ;)

cekk avatar Dec 01 '23 09:12 cekk

i've updated the code to work as @cekk explained before. @sneridagh @tiberiuichim @pnicolli @pbauer

giuliaghisini avatar Dec 04 '23 12:12 giuliaghisini