Skinr currently reads in CSS classes that can be defined in two ways.

  1. Correct: 'class' => array('grid16-1')
  2. Incorrect: 'class' => 'grid16-1'

It seems to me that it would be better coding practice that only the first way of writing be allowed.

CommentFileSizeAuthor
#5 skinr-class_arrays-1209938-5.patch2.63 KBmoonray
#4 patch_commit_2b37b313dc8e.patch1.05 KBmoonray

Comments

Hari’s picture

Hari commented

Should have been a little bit more specific. The above are examples of the two ways that a person could specify a class/classes in a skin.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Active

That's true, the second variant should not be supported. D7 always uses an array for the value of 'class'.

jacine’s picture

jacine
she/her
Location New York
commented

I thought we fixed this already.

moonray’s picture

moonray commented
Title: Skinr reads in CSS classes defined improperly/poorly coded. » Skinr allows CSS classes defined as string rather than array in a skin
Status: Active » Needs review
StatusFileSize
new patch_commit_2b37b313dc8e.patch1.05 KB

Attached patch triggers a warning in the logs rather than automatically fixing when a class array is given as a string instead.
Hopefully this is the right approach. :)

moonray’s picture

moonray commented
StatusFileSize
new skinr-class_arrays-1209938-5.patch2.63 KB

Attached patch adds tests.

moonray’s picture

moonray commented
Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

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