Following up on #1812048-11: Build the exposed form using form API functions, Views should use drupal_add_library to attach CSS/JS to form elements.

Files: 
CommentFileSizeAuthor
#18 2005616-18-use_attached_library.patch1.99 KBmikeker
PASSED: [[SimpleTest]]: [MySQL] 57,664 pass(es).
[ View ]
#13 2005616-13-use_attached_library.patch1.98 KBmikeker
PASSED: [[SimpleTest]]: [MySQL] 55,429 pass(es).
[ View ]
#10 2005616-9-use_attached_librarry.patch2.82 KBmikeker
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2005616-9-use_attached_librarry.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#5 2005616-5-do-not-test.patch2.82 KBmikeker
#2 2005616-2-do-not-test.patch2.74 KBmikeker
#1 2005616-1.patch2.74 KBmikeker
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2005616-1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

mikeker’s picture

Status:Active» Needs review
StatusFileSize
new2.74 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2005616-1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

The attached patch makes this change, however it requires the patch in #1812048-9: Build the exposed form using form API functions to be applied first.

mikeker’s picture

StatusFileSize
new2.74 KB

No need to make the testbot do unnecessary work...

tim.plunkett’s picture

+++ b/core/modules/views/lib/Drupal/views/ViewExecutable.phpundefined
@@ -442,7 +442,7 @@ public function __construct(ViewStorageInterface $storage) {
-    $this->element['#attached']['css'][] = drupal_get_path('module', 'views') . '/css/views.base.css';
+    drupal_add_library('views', 'views.base');

+++ b/core/modules/views/views.theme.incundefined
@@ -1054,7 +1054,7 @@ function template_preprocess_views_exposed_form(&$vars) {
-  @$form['#attached']['css'][] = drupal_get_path('module', 'views') . '/css/views.exposed_form.css';
+  drupal_add_library('views', 'views.exposed-form');

We do not want to use drupal_add_library at all.

What we can do is

$this->element['#attached']['library'][] = array('views', 'views.base');
+++ b/core/modules/views/views.moduleundefined
@@ -844,13 +844,18 @@ function views_hook_info() {
+    'css' => array("$path/css/views.base.css"),

@@ -865,13 +870,18 @@ function views_library_info() {
+    'css' => array("$path/css/views.exposed_form.css"),

Can you split this onto multiple lines please?

tim.plunkett’s picture

Title:Views should use drupal_add_library rather than #attached» Views should use ['#attached']['library'] rather than ['#attached']['css']
mikeker’s picture

StatusFileSize
new2.82 KB

Got it. Thanks for the clarification.

Edit: Also breaks css => array() into multiple lines, as requested.

Status:Needs review» Needs work

The last submitted patch, 2005616-1.patch, failed testing.

tim.plunkett’s picture

Status:Needs work» Needs review
dawehner’s picture

Status:Needs review» Reviewed & tested by the community

Thank you very much!

catch’s picture

Status:Reviewed & tested by the community» Needs work

Why the do not test patches? Please let the test bot run through just for sanity.

mikeker’s picture

Status:Needs work» Needs review
StatusFileSize
new2.82 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2005616-9-use_attached_librarry.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I'm pretty sure this patch will not apply until the patch in #1812048: Build the exposed form using form API functions is committed. Is there a better way to handle inter-related issues like this? Should I write this patch to only change the code already in 8.x and reroll #1812048: Build the exposed form using form API functions once this is checked in?

I figured issues with patches that didn't pass the testbot wouldn't get looked at by the powers-that-be so I added the do-not-test suffix. Regardless, let's see what the testbot says...

Status:Needs review» Needs work

The last submitted patch, 2005616-9-use_attached_librarry.patch, failed testing.

dawehner’s picture

Oh damn, you are right.

Btw. this patch misses the new css file.

mikeker’s picture

Status:Needs work» Needs review
StatusFileSize
new1.98 KB
PASSED: [[SimpleTest]]: [MySQL] 55,429 pass(es).
[ View ]

OK, slightly different approach... This patch only updates the existing CSS files to use ['#attached']['library'] rather than ['#attached']['css']. So there's no longer a dependency on any other issues.

I'll deal with updating the patch in #1812048: Build the exposed form using form API functions if/when this patch is committed.

Apologies for the back and forth -- I should've done it this way from the beginning.

Status:Needs review» Needs work

The last submitted patch, 2005616-13-use_attached_library.patch, failed testing.

mikeker’s picture

Status:Needs work» Needs review
dawehner’s picture

Status:Needs review» Reviewed & tested by the community
Issue tags:+VDC, +PHPUnit Blocker

Adding some tags.

alexpott’s picture

Status:Reviewed & tested by the community» Needs work

views.base.css is now views.module.css

mikeker’s picture

Status:Needs work» Needs review
StatusFileSize
new1.99 KB
PASSED: [[SimpleTest]]: [MySQL] 57,664 pass(es).
[ View ]

Chasing master...

damiankloip’s picture

Status:Needs review» Reviewed & tested by the community

Nice, those changes look OK to me!

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 9c173cf and pushed to 8.x. Thanks!

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