I needed this for something at work, it was generic enough that I decided to roll it as a patch to views directly.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Status: Active » Needs review
FileSize
1.85 KB
dawehner’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 7.x-3.x-dev » 8.x-dev
Component: Miscellaneous » views.module
Category: Feature request » Task
Status: Needs review » Patch (to be ported)
Issue tags: +VDC

D8 like.

tim.plunkett’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.69 KB

Woot!

jibran’s picture

Issue tags: +Need tests

We should add test for this as well.

damiankloip’s picture

Issue tags: -Need tests
FileSize
3.95 KB
2.26 KB

Here is a quick test.

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Tests/Handler/AreaMessageTest.php
@@ -0,0 +1,70 @@
+
+    // The handler is configured to show with empty views by default, so should
+    // appear.
+    $this->assertEqual(array('#theme' => 'status_messages'), $view->display_handler->handlers['empty']['messages']->render(), 'The expected render array was returned.');
+
+    // Turn empty off, and make sure it isn't rendered.
+    $view->display_handler->handlers['empty']['messages']->options['empty'] = FALSE;
+    // $empty parameter passed to render will still be FALSE, so should still
+    // appear.
+    $this->assertEqual(array('#theme' => 'status_messages'), $view->display_handler->handlers['empty']['messages']->render());
+    // Should now be empty as both the empty option and parameter are empty.
+    $this->assertEqual(array(), $view->display_handler->handlers['empty']['messages']->render(TRUE));
+  }

If we test such low level we could use actual unit tests right from the beginning. I don't see much of integration going on here. What do you think?

damiankloip’s picture

Yeah, sure. I just used an existing test and made it what I wanted :)

This could be a real unit test pretty easily. Leave it with me.

damiankloip’s picture

FileSize
3.82 KB
2.13 KB

Here we go.

dawehner’s picture

  1. +++ b/core/modules/views/tests/Drupal/views/Tests/Plugin/area/MessagesTest.php
    @@ -0,0 +1,77 @@
    +   *
    +   * @covers ::render()
    +   */
    

    should we also add defineOptions here?

  2. +++ b/core/modules/views/tests/Drupal/views/Tests/Plugin/area/MessagesTest.php
    @@ -0,0 +1,77 @@
    +    $this->assertsame(array('#theme' => 'status_messages'), $this->messagesHandler->render());
    

    you should really autocomplete more

  3. +++ b/core/modules/views/tests/Drupal/views/Tests/Plugin/area/MessagesTest.php
    @@ -0,0 +1,77 @@
    +
    

    let's skip that empty line

damiankloip’s picture

FileSize
3.84 KB
1.06 KB

Sorry, forgot about this issue. I guess everyone else did too. Either that or no one cares :)

Status: Needs review » Needs work

The last submitted patch, 10: 2210755-10.patch, failed testing.

damiankloip’s picture

10: 2210755-10.patch queued for re-testing.

Not sure I believe all of those fails.

damiankloip’s picture

Status: Needs work » Needs review
dawehner’s picture

Status: Needs review » Reviewed & tested by the community

We do care, but we just care about more than one issue

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 2210755-10.patch, failed testing.

tim.plunkett’s picture

10: 2210755-10.patch queued for re-testing.

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 2210755-10.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community

ImageStylesPathAndUrlTest--

alexpott’s picture

10: 2210755-10.patch queued for re-testing.

alexpott’s picture

I'm confused - wouldn't this mean that the messages would be moved from one location to another if the theme has an area for displaying messages? Like what is the use-case?

tim.plunkett’s picture

The use case I had was a view loaded in a modal, the messages were displaying on the page underneath, and I needed them right alongside the view.

On a whim I submitted it as a D7 views patch (which was committed), and this issue just exists to keep feature parity.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 04dd275 and pushed to 8.x. Thanks!

  • Commit 04dd275 on 8.x by alexpott:
    Issue #2210755 by damiankloip, tim.plunkett: Add an area handler to...

Status: Fixed » Closed (fixed)

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