Views should use ['#attached']['library'] rather than ['#attached']['css'] [#2005616]

Comment	File	Size	Author
#18	2005616-18-use_attached_library.patch	1.99 KB	mikeker

#13	2005616-13-use_attached_library.patch	1.98 KB	mikeker

#10	2005616-9-use_attached_librarry.patch	2.82 KB	mikeker

#5	2005616-5-do-not-test.patch	2.82 KB	mikeker

#2	2005616-2-do-not-test.patch	2.74 KB	mikeker

#1	2005616-1.patch	2.74 KB	mikeker

Comment #1

mikeker CreditAttribution: mikeker commented 28 May 2013 at 03:05

Status:

Active

» Needs review

File	Size
2005616-1.patch	2.74 KB

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.

Log in or register to post comments

Comment #2

mikeker CreditAttribution: mikeker commented 28 May 2013 at 03:06

File	Size
2005616-2-do-not-test.patch	2.74 KB

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

Log in or register to post comments

Comment #3

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 28 May 2013 at 03:40

+++ 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?

Log in or register to post comments

Comment #4

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 28 May 2013 at 03:41

Title:

Views should use drupal_add_library rather than #attached

» Views should use ['#attached']['library'] rather than ['#attached']['css']

Log in or register to post comments

Comment #5

mikeker CreditAttribution: mikeker commented 28 May 2013 at 05:01

File	Size
2005616-5-do-not-test.patch	2.82 KB

Got it. Thanks for the clarification.

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

Log in or register to post comments

Comment #6

28 May 2013 at 12:06

Status:

Needs review

» Needs work

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

Log in or register to post comments

Comment #7

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 28 May 2013 at 15:59

Status:

Needs work

» Needs review

Log in or register to post comments

Comment #8

dawehner

German

CreditAttribution: dawehner commented 30 May 2013 at 08:10

Status:

Needs review

» Reviewed & tested by the community

Thank you very much!

Log in or register to post comments

Comment #9

catch

he/him

English

CreditAttribution: catch commented 5 June 2013 at 11:50

Status:

Reviewed & tested by the community

» Needs work

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

Log in or register to post comments

Comment #10

mikeker CreditAttribution: mikeker commented 6 June 2013 at 15:17

Status:

Needs work

» Needs review

File	Size
2005616-9-use_attached_librarry.patch	2.82 KB

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...

Log in or register to post comments

Comment #11

6 June 2013 at 15:18

Status:

Needs review

» Needs work

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

Log in or register to post comments

Comment #12

dawehner

German

CreditAttribution: dawehner commented 6 June 2013 at 17:02

Oh damn, you are right.

Btw. this patch misses the new css file.

Log in or register to post comments

Comment #13

mikeker CreditAttribution: mikeker commented 7 June 2013 at 20:15

Status:

Needs work

» Needs review

File	Size
2005616-13-use_attached_library.patch	1.98 KB

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.