Comments

aheredia created an issue. See original summary.

aheredia’s picture

StatusFileSize
new3.71 KB

Here is the patch

aheredia’s picture

StatusFileSize
new15.81 KB
new16.87 KB
new24.95 KB
new18.95 KB

I've attached some screen shoots to clarify.

aheredia’s picture

aheredia’s picture

Status: Active » Needs review
miro_dietiker’s picture

Status: Needs review » Needs work

Awesome, let's get this in.

But a littlebit of test coverage (one single desc, one single assert) is a requirement. :-)

aheredia’s picture

StatusFileSize
new5.04 KB

Ok thanks

Here is the patch with the test coverage.

aheredia’s picture

Status: Needs work » Needs review
toncic’s picture

Status: Needs review » Needs work

Tested this and works fine. Just small improvements:

  1. +++ b/src/Entity/ParagraphsType.php
    @@ -156,6 +164,14 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
    +
    

    One extra blank line.

  2. +++ b/src/Tests/Classic/ParagraphsTypesTest.php
    @@ -73,4 +73,30 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
    +    $this->assertText("Saved the $label Paragraphs type.");
    

    We can add here one more assert to check if $description is displayed in 'DESCRIPTION' column after saving.

aheredia’s picture

Ok for the first

For the second I don't know exactly how to implement that. Could you give me some guidance on how to do it?

Thanks in advance.
Regards

aheredia’s picture

StatusFileSize
new5.04 KB

This is the patch

aheredia’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 11: description-2873576-11.patch, failed testing.

aheredia’s picture

Status: Needs work » Needs review

The test is correct. Retesting and passing the test again

aheredia’s picture

StatusFileSize
new5.33 KB

Finally I managed to do it.

Here is the patch with the new test.

aheredia’s picture

StatusFileSize
new964 bytes

Added the interdiff betweent #7 #15

VladimirMarko’s picture

Status: Needs review » Needs work
  1. +++ b/src/ParagraphsTypeInterface.php
    @@ -53,6 +53,14 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
       /**
    +   * Gets the description.
    +   *
    +   * @return string
    +   *   The description of this node type.
    +   */
    +  public function getDescription();
    

    The @return documentation should be The description of this paragraphs type..

  2. +++ b/src/Tests/Classic/ParagraphsTypesTest.php
    @@ -73,4 +73,34 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
    +    //Check if description is at Description column
    +    $header = count($this->xpath('//table/thead/tr/th[.="Description"]/preceding-sibling::th'));
    +    $row = count($this->xpath('//table/tbody/tr/td[.="' . $description . '"]/preceding-sibling::td'));
    +    $this->assertEqual($header, $row);
    

    This checks that the description is under the description header, if both exists.
    But it also passes, if there is no description and no description header.

    What we need is a simple

    $this->assertText("Description");
    $this->assertText($description);
    

    .

    Also, maybe it's better to rename $header to $header_position and $row to $row_position, or similar, to make it clearer in the variable names what is actually being checked here.

aheredia’s picture

StatusFileSize
new5.4 KB
new1.37 KB

Ok thanks.

Suggestions followed

aheredia’s picture

Status: Needs work » Needs review
VladimirMarko’s picture

Status: Needs review » Reviewed & tested by the community

I think this looks good.

Thank you for your work!

miro_dietiker’s picture

Status: Reviewed & tested by the community » Fixed

Very nice, committed. :-)
Now let's add those descriptions to the existing demo / collection paragraph types.

aheredia’s picture

Ok I'll try to go for them.

By the way I don't agree with author of the commit, i have made all the patches :(

miro_dietiker’s picture

@aheredia I attributed VladimirMarko and toncic for reviewing the patches and giving feedback. That's a key type of contribution.
Even if you would provide 100% perfect patches and someone does a QUALIFIED review with proper statement and setting it to RTBC, i would attribute that person.

I seem to accidentally have switched the patch author to VladimirMarko, i intended to keep you there.
To "fix" authoring, i pushed an empty commit with the proper author setting, so it can show up in your commit list.

aheredia’s picture

I agree 100% with you.
Many thanks for the clarification.

Regards

Status: Fixed » Closed (fixed)

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