Skip to content
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

pat-contentbrowser implementation #1377

Merged
merged 51 commits into from
Sep 18, 2024
Merged

pat-contentbrowser implementation #1377

merged 51 commits into from
Sep 18, 2024

Conversation

petschki
Copy link
Member

@petschki petschki commented Mar 14, 2024

This is the pat-relateditems replacement using a miller column content browser.

Feature completeness:

  • Multiple pat-contenbrowser occurrences on a single page
  • TinyMCE LinkModal implementation
  • filtering current level list
  • maximumSelectionSize
  • pat-upload to current level
  • selectable levels
  • batchable levels
  • drag&drop sortable items
  • selectableTypes
  • translations
  • multiple items select in column browser
  • keyboard navigation
  • TESTS
  • favorites menu
  • recently used menu
  • use @plone/registry to allow overriding components

Test dependencies:

Regarding search mode:
we decided to not implement this here. The pat-select2 pattern is better for this usecase.
UPDATE: Since this should be a feature complete replacement for pat-relateditems, the search mode is available too.

Sneak Preview (updated 2024-05-16)

TinyMCE Link/Image

tinymce.link.image.mov

Related items selection and resorting

single.select.resort.mov

Multiselection with shift or ctrl/cmd key

multiselect.shift.cmd.mov

Keyword filtering

keyword.filter.mov

Uploading

upload.mov

@thet
Copy link
Member

thet commented Mar 17, 2024

looking at the video, great!
how can this be tested?
are there branches in plone repos using contentbrowser instead of relateditems?
or should we maybe provide an alias "relateditems" for "contentbrowser"?
i think this also needs some integration in docs.

@petschki
Copy link
Member Author

petschki commented Mar 17, 2024

This can be tested by using the mentioned branches of p.a.z3cform and p.a.relationfield and the webpack devel resources in the resource registry. The relationfield uses the contentbrowser widget already.

Updating all the core packages which use RelatedItemsFieldWidget isn't that hard but there are several addons which have to be updated too ... I'm not sure if its a problem when we "alias" RelatedItemsFieldWidget to ContentBrowserFieldWidget under the hood. Maybe that does work for the addons. I can give this a test ...

One more benefit from removing pat-relateditems is that we can get rid of the patched select2 3.5.x version.

@thet
Copy link
Member

thet commented Mar 18, 2024

Cool! Sorry I didn't read the ticket description carefully enough.

@petschki
Copy link
Member Author

@MrTango @thet I tried to figure out the jest setup for svelte apps using this link https://www.roboleary.net/2021/11/18/svelte-app-testing-jest.html ... now I get the following message:

Jest is being called in CJS mode. You must use ESM mode in Svelte 4+

Maybe someone of you can give me a hint how to setup the testing environment?

@petschki petschki force-pushed the pat-content-browser-upload branch from 2db6cd7 to 197d73f Compare March 26, 2024 06:46
Copy link
Member

@thet thet left a comment

Choose a reason for hiding this comment

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

@MrTango @petschki great work so far!

All in all this LGTM so far.

Some remarks though:

  • Sorting doesn't seem to work. I see there is some sorting code in SelectedItems.svelte but for me adding multiple items and trying to sort just doesn't work.

Some feature requests, which are irrelevant to this PR but anyhow:

  • Multiple selection would be nice. Currently you can only select one by one.

  • I'd love some kind of keyboard navigation would be great. Like Arrow navigation, enter to move in a lower directory and setting the focus in that directory, enter to select a node.

  • Using plone.restapi instead of custom endpoints. IMO this is something we really should go for. Why would we have custom JSON endpoints implemented when we can have everything and more through plone.restapi? But for the first iteration depending on the custom JSON endpoints like pat-relateditems is perfectly OK.

@thet
Copy link
Member

thet commented Apr 3, 2024

@MrTango @petschki About sorting the selected items: What do you think about using pat-sortable from Patternslib? pat-sortable would need to be extended to make use of MutationObservers first. Currently the reinitialization only happens when a pat-update event is thrown with a added payload, which couples that pattern too tightly to internals of Patternslib, this is why I‌ think a MutationObserver should be used instead.

Ref: Patternslib/Patterns#1197

@petschki
Copy link
Member Author

petschki commented Apr 4, 2024

@thet Thanks for your review!

About sorting: this only gets initialized on loading the first time and doesn't work during "first time" item selection indeed. (https://github.com/plone/mockup/pull/1377/files#diff-4b241e899bfbe65a8c35ffadf58f567fea3d4df013c56306db87d1517e10798dR83) this is a small change and I will fix it asap. I'm not sure if we should use pat-sortable for this usecase.

Features:

  • Multiselection is definitely on my list to implement.
  • keyboard navigation too
  • and restapi: yes, but this needs some more thoughts about "inplace" replacement of pat-relateditems and its options. We often use relateditems pattern as mode="search" with some additional index like portal_type=["Image", "File"] ... IMHO this should work here too ... and it's not clear to me which endpoint we should use here.

@petschki petschki force-pushed the pat-content-browser-upload branch 4 times, most recently from bfcd796 to b79cc0c Compare April 5, 2024 10:47
@MrTango
Copy link
Contributor

MrTango commented Apr 10, 2024

regarding keyboard navigation, yes definitely, i think i had some key's already implemented like ESC to close the browser, but i had to disabled some stuff, when i did update to Svelte 4. Not sure if is enabled already. That was a plugin.
But yeah, i want all the keyboard navigation we have in relateditems widget now and more.

regarding RestApi, here from today's classic UI meeting:

  • should we use plone.restapi for the JSON endpoints?
    • talking with @MrTango, @jensens, @petschki we should do not because it should be possible to use Plone classic UI without restapi (jens mentioned circular dependency problems.)
    • the vocabulary end point in Plone core has all features we need.

@MrTango
Copy link
Contributor

MrTango commented Apr 10, 2024

and also multiselection is already on the list, even if we didn't had it before in relateditems.

@MrTango
Copy link
Contributor

MrTango commented Apr 10, 2024

btw the sorting code, we have right now is for sorting the selected items, not for sorting the items in the browser ;)
But sorting in a column would be nice to have.
Regarding global search in content browser?
- we decided to not do it, one can use ajax select widget to have this, no need to also have it here

@petschki
Copy link
Member Author

I do not think the levels should be sortable. thats too much magic for this package.

The problem was, that initially when there are no selectedItems and you added some, they weren't sortable immediately in the field ... I've fixed that here c264843

+1 for using a different pattern for search mode. AjaxSelectFieldWidget would fit best for this.

@petschki petschki force-pushed the pat-content-browser-upload branch 2 times, most recently from 3463af6 to ecb8161 Compare April 25, 2024 22:10
@petschki petschki force-pushed the pat-content-browser-upload branch 2 times, most recently from 1e2491b to 7f5108d Compare May 15, 2024 11:12
@petschki petschki force-pushed the pat-content-browser-upload branch from 32e736e to 14512c6 Compare September 18, 2024 08:32
@petschki
Copy link
Member Author

OK ... I'll merge the current state and release another alpha of that.

Next week I'll work on :

  • favorites menu
  • recently used menu

and eventually :

  • use @plone/registry to allow overriding components

@petschki petschki merged commit 483c2bb into master Sep 18, 2024
3 checks passed
@petschki petschki deleted the pat-content-browser-upload branch September 18, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

PLIP: Add contentbrowser as a replacement for relateditems
4 participants