some clean up/improvements in block example module:

  • make BlockExampleController extend the ControllerBase and use its methods
  • add title to block_example.description route
  • add leading / in path for block_example.description route
  • add use statement for Drupal\Core\Block\BlockPluginInterface in the block_example.module
  • remove unused/obsolete use statements in Example*Block classes
  • replace t() usage with $this->t() in Example*Block classes
  • change block_example_string_text type from textfield to textarea for better redabilty in ExampleConfigurableTextBlock::blockForm()

Comments

willzyx’s picture

StatusFileSize
new6.88 KB

mile23’s picture

Status: Needs review » Needs work

Hey, looks great, thanks.

Just one thing:

+++ b/block_example/src/Tests/BlockExampleTest.php
@@ -26,23 +26,21 @@ class BlockExampleTest extends WebTestBase {
   public function setUp() {
     parent::setUp();
     // Create user.
-    $this->webUser = $this->drupalCreateUser(array('administer blocks'));
+    $web_user = $this->drupalCreateUser(array('administer blocks'));
+    // Login the admin user.
+    $this->drupalLogin($web_user);
   }
...
   public function testBlockExampleBasic() {

Since there's only one test method, let's just move the user creation and login into the test method, and just delete our setUp() override.

willzyx’s picture

Status: Needs work » Needs review
StatusFileSize
new6.88 KB
new886 bytes

Status: Needs review » Needs work

The last submitted patch, 4: block_example_clean-2497625-2.patch, failed testing.

willzyx’s picture

Status: Needs work » Needs review

copy(/var/lib/drupaltestbot/sites/default/files/checkout/sites/simpletest/487467/settings.php): failed to open stream: No such file or directorycopy('/var/lib/drupaltestbot/sites/default/files/checkout/sites/default/default.settings.php', '/var/lib/drupaltestbot/sites/default/files/checkout/sites/simpletest/487467/settings.php')

mile23’s picture

What are those error messages about?

willzyx’s picture

What are those error messages about?

are about the failing test in #5

mile23’s picture

Ah, OK, I see. Thanks.

mile23’s picture

I can't get #4 to pass locally, so I'll try the testbot here.

Note that there's a branch blocker on block_example that I'm trying to fix, so that might represent an API change not present in the code base or in the patch.

Status: Needs review » Needs work

The last submitted patch, 4: block_example_clean-2497625-2.patch, failed testing.

willzyx’s picture

Status: Needs work » Needs review
StatusFileSize
new7.28 KB

Mile23 #2512456: Implement the new block layout design to emphasize the primary interaction of placing a block changes a bit the block layout page. The attached patch should solves the issue with test

Note that there's a branch blocker on block_example that I'm trying to fix, so that might represent an API change not present in the code base or in the patch.

there is a open issue? if you are interested I can help

willzyx’s picture

StatusFileSize
new761 bytes

the interdiff

Status: Needs review » Needs work

The last submitted patch, 14: block_example_clean-2497625-14.patch, failed testing.

willzyx’s picture

Status: Needs work » Needs review
StatusFileSize
new6.88 KB

@Mile23 you just committed the change that I did in the test :) . This is a simple reroll

mile23’s picture

Status: Needs review » Fixed

Thanks, @willzyx. Generally branch blockers don't get issues unless they're majorly crazy or need community input. In this case it was just the URL change.

This looks like all best-practices stuff, which is good. Committed and pushed.

  • Mile23 committed 59be420 on 8.x-1.x authored by willzyx
    Issue #2497625 by willzyx: Block example clean up/improvements
    

Status: Fixed » Closed (fixed)

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