Overview

This is follow-up to #3539729.

  1. When new SDC or block based Component entity is created, we should check if a Folder matching group property of such SDC exists. If it does, place Component in that Folder. If such a Folder doesn't exist, create it.
    → partially handled in #3541364: Delete folders — that handled auto-creation for Components, not yet for JavaScriptComponents.
    → remainder handled in #3549726: Remove `category` from `Component` config entities, add `ComponentSourceInterface::determineDefaultFolder()` instead
  2. 🆕 since #9: update JsComponent::createConfigEntity() to NOT set category: @todo for the Component for an exposed code component, because this causes a "@todo" Folder to be created. → handled in #3549726: Remove `category` from `Component` config entities, add `ComponentSourceInterface::determineDefaultFolder()` instead
  3. When User deletes a Folder, we should disable (status: false) all of Component entities it "contains", or reject deleting the folder if any of the components are currently in use.

We also need tests for all of the above.

Acceptance Criteria

  1. Prevent deletion of folders containing any components.
  2. Allow deletion of empty folders.
  3. When the Delete Folder option is disabled, display a tooltip on hover to inform the user that folders with components cannot be deleted.
  4. Disable the Delete Folder option when the folder contains any components.
CommentFileSizeAuthor
#23 delete folders.mov32.39 MBvipin.mittal18

Issue fork canvas-3541364

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

f.mazeikis created an issue. See original summary.

balintbrews’s picture

balintbrews
he/him/his
Location Netherlands 🇳🇱
commented
Title: Expand Folder entity functionality » Auto-create folders & disable components when folders get deleted
Category: Task » Feature request
Issue summary: View changes
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Auto-create folders & disable components when folders get deleted » [PP-1] Auto-create folders & disable components when folders get deleted
Status: Active » Postponed
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [PP-1] Auto-create folders & disable components when folders get deleted » Auto-create folders & disable components when folders get deleted
Status: Postponed » Active

#3539729: Implement Folder config entity is in!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes
Related issues: +#3541364: Delete folders
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Auto-create folders & disable components when folders get deleted » Disable components when folders get deleted

Re-scoping per #5.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Disable components when folders get deleted » Disable Components when Folders get deleted
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Disable Components when Folders get deleted » Auto-create Folders for code components & disable Components when Folders are deleted
Issue summary: View changes
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes

Paving the path for #3543533: [PP-1] Display both internal and exposed code components in "Manage library" led me to install the test-only module that provides status: true code components. Which in turn surfaced a bug more related to this issue than #3543533: [PP-1] Display both internal and exposed code components in "Manage library".

Doing that surfaced an interesting consequence of a loose end from back in January: JsComponent::createConfigEntity() always sets category: @todo, which now results in a "@todo" Folder getting created for Components, whenever a code component changes from status: false to status: true. Updated test expectations: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

Project: Experience Builder » Drupal Canvas

Experience Builder has been renamed to Drupal Canvas in preparation for its beta release. You can now track issues on the new project page.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » balintbrews
Status: Active » Postponed (maintainer needs more info)
Issue tags: +Needs issue summary update

Point 2 in the issue summary has already been implemented in #3545716: Disable Components by default sourced from some of the block plugins provided by core in the Standard profile, and stop using `@todo` as the category/Folder for code components, at the request of @balintbrews at #3545716-8: Disable Components by default sourced from some of the block plugins provided by core in the Standard profile, and stop using `@todo` as the category/Folder for code components.

What's not yet clear to me is how code components can ever have a pre-defined category, which then would determine their Folder. Would that be a separate piece of metadata in the code component editor UI that is currently missing? Or … is the intent to not have that at all anymore, since why even ask that if the code component can be published and then assigned to whichever Folder?

Tangentially related: #3549726: Remove `category` from `Component` config entities, add `ComponentSourceInterface::determineDefaultFolder()` instead.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Auto-create Folders for code components & disable Components when Folders are deleted » [PP-1] Disable Components when Folders are deleted
Assigned: balintbrews » Unassigned
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Postponed
Issue tags: -Needs issue summary update

Per #3549726-18: Remove `category` from `Component` config entities, add `ComponentSourceInterface::determineDefaultFolder()` instead, that issue will have implemented #11!

That makes the remaining scope here smaller still.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes
vipin.mittal18’s picture

vipin.mittal18
He/Him
Primary language English
Location Greater Noida, India
commented
Title: [PP-1] Disable Components when Folders are deleted » [PP-1] Delete folders
Component: Config management » Code
Issue summary: View changes
Status: Postponed » Needs work

The requirement has changed to simply delete the folder, with no activity for disable. Hence added Acceptance Criteria

lauriii made their first commit to this issue’s fork.

lauriii opened merge request !516

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Title: [PP-1] Delete folders » Delete folders
Status: Needs work » Needs review
utkarsh_33’s picture

utkarsh_33 commented
Status: Needs review » Needs work

Overall changes looks good to me just a small suggestion of adding a toggle tip on the disabled button would be a better UX. Marking it NW for some small changes.

utkarsh_33’s picture

utkarsh_33 commented
Status: Needs work » Needs review

Added the toggle tip. Marking it NR again.

utkarsh_33’s picture

utkarsh_33 commented
Status: Needs review » Needs work

I think this is missing tests. Moving it back to NW.

utkarsh_33’s picture

utkarsh_33 commented
Status: Needs work » Needs review

I have added test coverage as well. This is ready for review again.

vipin.mittal18’s picture

vipin.mittal18
He/Him
Primary language English
Location Greater Noida, India
commented
Status: Needs review » Needs work
vipin.mittal18’s picture

vipin.mittal18
He/Him
Primary language English
Location Greater Noida, India
commented
Issue summary: View changes
StatusFileSize
new delete folders.mov32.39 MB
utkarsh_33’s picture

utkarsh_33 commented
Status: Needs work » Needs review
kunal.sachdev’s picture

kunal.sachdev commented
Status: Needs review » Needs work

I was testing this for the case "When the Delete Folder option is disabled, display a tooltip on hover to inform the user that folders with components cannot be deleted." and I noticed a slight delay before the tooltip appears; I believe it should appear instantly on hover for a snappier user experience. Can we fix this?

utkarsh_33’s picture

utkarsh_33 commented

Re #25.

We can use <Tooltip> component instead of native HTML title attribute but that would require some additional css for it's styling. So we need to decide if that's really important.
What currently happens is the title attribute on the UnifiedMenu.Item component is using the native HTML title attribute, which has a built-in delay before the tooltip appears.

vipin.mittal18’s picture

vipin.mittal18
He/Him
Primary language English
Location Greater Noida, India
commented
Status: Needs work » Needs review

Re #25.

Yes, this is acceptable. The delay is normal for native HTML tooltips as per browser behaviour, and no changes are required unless a custom tooltip experience is desired.

kunal.sachdev’s picture

kunal.sachdev commented
Status: Needs review » Reviewed & tested by the community

That makes sense. If using the <Tooltip> component introduces significant CSS overhead just for this one instance, I understand sticking with the native attribute for now. Since the functionality itself works as intended, I'm moving this to RTBC!!

  • narendrar committed da529104 on 1.x authored by lauriii 
    feat: #3541364 Delete folders

By: f.mazeikis
By: balintbrews
By: wim...
narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.