Problem/Motivation

'Collapse' action does validation for whole form, that is not what we want. Check how 'Duplicate' works.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#14 collapse_button-2867385-14.patch3.69 KBGinovski
#14 interdiff-2867385-11-14.txt1.09 KBGinovski
#11 interdiff-2867385-6-11.txt1.53 KBhkoch
#11 collapse_button_does-2867385-11.patch2.6 KBhkoch
#6 collapse_button_does-2867385-6.patch3.64 KBtbonomelli
#6 interdiff-2867385-5-6.txt2.01 KBtbonomelli
#5 collapse_button_does-2867385-5.patch3.41 KBtbonomelli
#5 interdiff-2867385-3-5.txt2.03 KBtbonomelli
#3 collapse_button_does-2867385-3.patch1.38 KBtbonomelli
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

toncic created an issue. See original summary.

tbonomelli’s picture

tbonomelli CreditAttribution: tbonomelli at MD Systems GmbH commented
Assigned: Unassigned » tbonomelli
tbonomelli’s picture

tbonomelli CreditAttribution: tbonomelli at MD Systems GmbH commented
Status: Active » Needs review
FileSize
collapse_button_does-2867385-3.patch1.38 KB

Added limit_validation_errors to the collapse button.

Status: Needs review » Needs work

The last submitted patch, 3: collapse_button_does-2867385-3.patch, failed testing.

tbonomelli’s picture

tbonomelli CreditAttribution: tbonomelli at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
interdiff-2867385-3-5.txt2.03 KB
collapse_button_does-2867385-5.patch3.41 KB

Removed now unnecessary lines of code which caused the test to fail.

tbonomelli’s picture

tbonomelli CreditAttribution: tbonomelli at MD Systems GmbH commented
FileSize
interdiff-2867385-5-6.txt2.01 KB
collapse_button_does-2867385-6.patch3.64 KB

Readded the removed code lines and altered them so they don't fail anymore.

Instead of validating the form with the Collapse button it uses the 'save and keep published' button.

Primsi’s picture

Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commented

Ok, tested this a bit,... and if we remove this validation, we allow saving of entities that are missing required fields. This is definitely not what we want.

I was thinking a bit about other possible solutions, but I don't see one, so I am inclined to the idea to close this issue if we don't have other proposals.

toncic’s picture

toncic CreditAttribution: toncic at MD Systems GmbH commented
Status: Needs review » Needs work

We should have the same functionality as a duplicate button.
I tested a little bit with this
'#limit_validation_errors' => [array_merge($parents, [$field_name, 'add_more'])],
and seems works properly.

Primsi’s picture

Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commented

Meh, ... somehow I managed to overlook that we in fact want to validate just the current paragraph, @toncic is right. Ignore my previous comment :P

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented
Priority: Normal » Major

Promoting since i have seen severe ux issues with users fighting against required field errors while they want to interact with adding / removing / editing=expand|collapse elements..

hkoch’s picture

hkoch CreditAttribution: hkoch as a volunteer commented
Assigned: tbonomelli » hkoch
Status: Needs work » Needs review
FileSize
collapse_button_does-2867385-11.patch2.6 KB
interdiff-2867385-6-11.txt1.53 KB

Applied suggestion from comment #8

Status: Needs review » Needs work

The last submitted patch, 11: collapse_button_does-2867385-11.patch, failed testing.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented

Drupal\paragraphs\Tests\Classic\ParagraphsAdministrationTest 507 passes 1 fails

Ginovski’s picture

Ginovski CreditAttribution: Ginovski at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
interdiff-2867385-11-14.txt1.09 KB
collapse_button-2867385-14.patch3.69 KB

Fixed the administration test.

  • miro_dietiker committed e1e823c on 8.x-1.x authored by Ginovski 
    Issue #2867385 by tbonomelli, Ginovski, hkoch, toncic, Primsi: Collapse...
miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented
Status: Needs review » Fixed

Very nice. Committed.
One content editor pain less, party!

:-)

  • miro_dietiker committed 31fe6df on 8.x-1.x authored by hkoch 
    Issue #2867385 by tbonomelli, hkoch, Ginovski, toncic, Primsi: Collapse...
miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented

Oops, attributed the wrong person for patch creation, thus pushed a second empty commit. :-)

Status: Fixed » Closed (fixed)

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