Support from Acquia helps fund testing for Drupal Acquia logo

Comments

miro_dietiker created an issue. See original summary.

bartlangelaan’s picture

Status: Active » Needs review
FileSize
2.04 KB

This patch switches the default widget, and changes the title & descriptions of the widgets.

Didn't run any test, so let's see which errors come up!

bartlangelaan’s picture

Berdir’s picture

Status: Needs review » Needs work

Thanks!

A test would be useful that asserts that the stable one is chosen by default, I guess all our current tests currently specify it explicitly.

Also, we have a lot of *Experimental* test, we should probably rename them too, will result in some conflicts unfortunately. Probably not worth touching legacy/classic.

bartlangelaan’s picture

This replaces all references to the 'classic' and 'experimental' widget.

There is no new test for testing the default widget yet.

bartlangelaan’s picture

Berdir’s picture

  1. +++ b/.idea/$CACHE_FILE$
    @@ -0,0 +1,20 @@
    +<project version="4">
    +  <component name="ProjectInspectionProfilesVisibleTreeState">
    +    <entry key="Project Default">
    +      <profile-state>
    +        <expanded-state>
    +          <State>
    +            <id />
    +          </State>
    +          <State>
    

    looks like you added some .idea files to the patch.

  2. +++ b/src/Tests/LegacyWidget/ParagraphsTestBase.php
    @@ -1,6 +1,6 @@
    -namespace Drupal\paragraphs\Tests\Classic;
    +namespace Drupal\paragraphs\Tests\LegacyWidget;
     
    

    So core actually disallows to use Legacy prefixed tests o.0

    Another reason to keep them as they are as "Classic" then, for now anyway.

bartlangelaan’s picture

Status: Needs work » Needs review
FileSize
61.65 KB

For the last patch, I made a quick adjustment to check if LegacyWidget was okay when Legacy wasn't, but everything that starts with \Legacy is giving an error. I didn't have my tests set up locally so that's why I uploaded it, sorry for the trouble.

In this patch, I changed it to \WidgetLegacy, which should be allowed.

Also, I added a new test:
Drupal\Tests\paragraphs\Functional\ParagraphsUiTest::testDefaultWidget
It verifies that the 'paragraphs' widget is used by default.

Furthermore, cleaned up the patch.

Another reason to keep them as they are as "Classic" then, for now anyway.

I can revert the changes if you want, but they should work now. I think that we should keep the labels and test names consistent, so if you want the old widget to be marked as 'Legacy' as noted in the issue summary, we should also do that in the test names.

bartlangelaan’s picture

FileSize
62.71 KB

Re-roll!

Berdir’s picture

Berdir’s picture

Status: Needs review » Fixed
FileSize
3.21 KB
60.04 KB

Updating the readme description a bit.

miro_dietiker’s picture

Wohooo, let's celebrate this! :-)

shelane’s picture

What commit was this fixed in? I was still seeing “EXPERIMENTAL” in the edit form display screen after updating to 1.12.

  • Berdir committed 4f44470 on 8.x-1.x authored by bartlangelaan
    Issue #3080556 by bartlangelaan, Berdir: Declare the EXPERIMENTAL Widget...
Berdir’s picture

Hm, somehow the commit isn't there. Not sure what happened. I pushed it again now.

Status: Fixed » Closed (fixed)

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

mpp’s picture

The project page still lists the "experimental" and the "classic" widget. It should be updated with the same content as the readme.

The readme states:
" * Legacy (formerly Classic): a stable UI with limited features that will not be changed or updated."
Perhaps it would be more clear if the readme explicitly states that the classic widget is deprecated (and needs to be migrated to the stable widget)?