Problem/Motivation

Initially this issue was about changing the label of the contextual (delete) link to match the label in block-layout (remove). In fact the link in contextual links actually deletes the block and all of it's instanaces. This is too destructive an action to expose in a contextual link, since users are likely to think they are only removing a single instance when they are in fact removing all instances and any associated content.

Proposed resolution

Remove the 'delete' contextual link from blocks.

Remaining tasks

Create patch

User interface changes

There will be no 'delete' link on blocks.

API changes

?

Data model changes

?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tkoleary created an issue. See original summary.

laxman.ghavte’s picture

Assigned: Unassigned » laxman.ghavte
tkoleary’s picture

Priority: Minor » Normal
Issue tags: +Usability, +sprint
tkoleary’s picture

Issue tags: +Novice
tkoleary’s picture

Priority: Normal » Major
tim.plunkett’s picture

Status: Needs work » Active

Needs a patch.

dmezquia’s picture

Issue tags: -Usability, -sprint
tim.plunkett’s picture

Issue tags: +Usability, +sprint

IMO those were relevant

shubhang’s picture

As this issue has a "Novice" tag, so can you be a little more descriptive of where this change needs to be made.
I am a begginer and really wants to contribute to Drupal.

tkoleary’s picture

The file that needs to be changed is core/modules/block_content/block_content.links.contextual.yml

Ada Hernandez’s picture

Ada Hernandez’s picture

Status: Active » Needs review
tkoleary’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
25.85 KB

Looks good.

Note: the image above shows two "quick edit" links, that's being addressed in another issue.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: block_layout_action_now-2826728-11.patch, failed testing.

Thew’s picture

Assigned: laxman.ghavte » Unassigned
Status: Needs work » Reviewed & tested by the community

Patch passed.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: block_layout_action_now-2826728-11.patch, failed testing.

lomasr’s picture

FileSize
43.21 KB
23.64 KB

Applied the patch in #11 . Worked cleanly for me. Please see the screens.

boosmith’s picture

I'm looking to test this but I am not sure exactly which URL this is in Drupal as I can't reproduce the screenshots attached. Thanks

tkoleary’s picture

@boosmith

Best way to test it is to:

  1. copy the link address of the patch
  2. go to simplytest.me
  3. choose Drupal 8.3.x
  4. click 'add a patch'
  5. paste the patch address in the field
  6. Install Drupal
  7. Go to 'structure/block-layout/custom-block-library
  8. Add a new custom block
  9. Go back to structure/block-layout
  10. Click any one of the 'place block' links
  11. Place the block you just created
  12. Click 'Back to site'
  13. You should see your block, go to it's contextual link dropdown (upper right corner)

The reason for steps 7-13 is that system created blocks that are on the page by default do not have 'remove' (or delete) links.

boosmith’s picture

Thanks tkleary. I followed the steps in #19 and it passed testing.
Screen Shot 2017-01-10 at 20.43.12.png

utkarsh_malviya’s picture

After we click on the remove link the next page still shows delete.

tim.clifford’s picture

FileSize
16.6 KB

Can confirm patch #11 works against core 8.3.x but the next page does still say delete (#21)

utkarsh_malviya’s picture

@tkoleary can you help me solve this? I am new here.

tim.clifford’s picture

Changed contextual link and button on following page.

utkarsh_malviya’s picture

@tim.clifford the button on the next page also says delete!

tim.clifford’s picture

As utkarsh_malviya points out, the form submit button label needs to be updated as well. Patch #26 updates both the contextual filter and the delete button label. I have also added a space in between sentences for the message.

tim.clifford’s picture

Tested on 8.3.x and seems to be working fine.

Screenshot

tim.clifford’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 26: block_layout_action_now-2826728-26.patch, failed testing.

tim.plunkett’s picture

Except the big bold title says "delete" and the rest of the words say "remove"

tim.clifford’s picture

Updating test and re-applying patch.

tim.clifford’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 31: block_content_links_patch-31.patch, failed testing.

tim.plunkett’s picture

+++ b/core/modules/block_content/src/Form/BlockContentDeleteForm.php
@@ -17,11 +18,18 @@ public function buildForm(array $form, FormStateInterface $form_state) {
-      '#markup' => $this->formatPlural(count($instances), 'This will also remove 1 placed block instance.', 'This will also remove @count placed block instances.'),
+      '#markup' => $this->formatPlural(count($instances), 'This will also remove 1 placed block instance. ', 'This will also remove @count placed block instances. '),

Why this change?

Also
PHP Parse error: syntax error, unexpected ')' in /var/www/html/core/modules/block_content/src/Tests/BlockContentListViewsTest.php on line 106

tkoleary’s picture

@tim.plunkett

looks like it adds a space after "instances. "

tim.plunkett’s picture

Yes I can see *what* it is doing, I asked *why* :)

tim.clifford’s picture

Space was added as sentence that followed started immediately after the '.'

Screenshot

tim.clifford’s picture

Syntax error fix. Removed the space as mentioned above.

tim.clifford’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 38: block_content_links_patch-38.patch, failed testing.

tim.clifford’s picture

tim.clifford’s picture

tim.plunkett’s picture

@tim.clifford can you include interdiffs for your changes from now on? Thanks!
https://www.drupal.org/documentation/git/interdiff

tim.plunkett’s picture

+++ b/core/modules/block_content/block_content.links.contextual.yml
@@ -4,7 +4,7 @@ block_content.block_edit:
-  title: 'Delete'
+  title: 'Remove'

+++ b/core/modules/block_content/block_content.links.task.yml
@@ -17,7 +17,7 @@ entity.block_content.canonical:
-  title: Delete
+  title: Remove

+++ b/core/modules/block_content/block_content.routing.yml
@@ -30,7 +30,7 @@ entity.block_content_type.delete_form:
-    _title: 'Delete'
+    _title: 'Remove'

@@ -60,7 +60,7 @@ entity.block_content.delete_form:
-    _title: 'Delete'
+    _title: 'Remove'

+++ b/core/modules/block_content/src/Form/BlockContentDeleteForm.php
@@ -24,4 +25,11 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+  public function getConfirmText() {
+    return $this->t('Remove');
+  }

After trying to debug #2839558: Blocks do not have a "remove" contextual link, I realized that all of these changes are wrong.

None of these will just remove that placed block, they will delete the block content entity and ALL placements of the block. It is a destructive deletion of user-entered content, not a easily reversible configuration change.

Either this is won't fix, or the issue needs more clarification.

The title says it all.

Not quite :)

tkoleary’s picture

Title: Block layout action now says 'remove' but contextual link still says 'delete' » Block layout action removes instance, but contextual link deletes all instances.
Issue summary: View changes
tim.plunkett’s picture

Component: block.module » block_content.module
Status: Needs work » Needs review
Issue tags: -Needs issue summary update
FileSize
573 bytes
larowlan’s picture

+1 to just remove

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

rosschive’s picture

Status: Needs review » Reviewed & tested by the community

Taking a look as part of Global Sprint Weekend (Boston)...

I created 8.4.x sandbox with patch 46 (via simplytest.me). Verifying that following the placement of a custom block, the contextual editor options do not contain either a 'Remove' or 'Delete' option (see screenshot).

Only local images are allowed.

The last submitted patch, 42: block_content_links_patch-38.patch, failed testing.

  • webchick committed 969f525 on 8.4.x
    Issue #2826728 by tim.clifford, tim.plunkett, Adita, lomasr, tkoleary,...

  • webchick committed b70e432 on 8.3.x
    Issue #2826728 by tim.clifford, tim.plunkett, Adita, lomasr, tkoleary,...
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.4.x and backported to 8.3.x. Thanks!

xjm’s picture

Version: 8.4.x-dev » 8.3.x-dev
Issue tags: +8.3.0 release notes

Status: Fixed » Closed (fixed)

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