It's great that we can use a field to set the row class of a view display but it is literally putting the raw text in without making it CSS friendly.

e.g. I am using the term field to set the row class where my terms are "TTC Event", "TTC Performance", "External". These end up exactly like this in the generated CSS (with spaces...) rather than being converted to "ttc-event", "ttc-performance" and "external"

Files: 
CommentFileSizeAuthor
#49 views.code_.1360186-49.patch3.34 KBPol
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]
#47 views.code_.1360186-47.patch3.34 KBPol
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]
#46 views.code_.1360186-46.patch1.26 KBPol
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]
#45 views.code_.1360186-44.patch4.74 KBPol
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]
#41 views.code_.1360186-41.patch4.72 KBPol
FAILED: [[SimpleTest]]: [MySQL] 1,430 pass(es), 37 fail(s), and 6 exception(s).
[ View ]
#39 views.code_.1360186-39.patch6.75 KBPol
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]
#38 views.code_.1360186-38.patch5.72 KBPol
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]
#33 views-clean-row-classes-1360186-33.patch2.43 KBcodi
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]
#30 1360186-views-row-class-cleansing-2.patch1.37 KBdavidwhthomas
FAILED: [[SimpleTest]]: [MySQL] 1,617 pass(es), 10 fail(s), and 0 exception(s).
[ View ]
#24 1360186-24.patch1.38 KBGiorgosK
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in 1360186-24.patch.
[ View ]
#21 views-add-strtolower-1360186-21.patch1.4 KBjaroslaw.kaminski
FAILED: [[SimpleTest]]: [MySQL] 1,617 pass(es), 10 fail(s), and 0 exception(s).
[ View ]
#16 1360186-views-row-class-cleansing.patch1.39 KBdavidwhthomas
PASSED: [[SimpleTest]]: [MySQL] 1,472 pass(es).
[ View ]
#1 1360186-views-clean-row-classes.patch1.06 KBdavidwhthomas
PASSED: [[SimpleTest]]: [MySQL] 1,472 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.06 KB
PASSED: [[SimpleTest]]: [MySQL] 1,472 pass(es).
[ View ]

I've had the same issue.

Adding a row class wasn't properly escaped with the resulting class containing spaces

e.g class="directory-Special Node my author"

The attached patch fixes the issue, for multiple and single row classes.

e.g, classes become class="directory-Special-Node my-author"

Note, you can test by adding a row class to the format settings, e.g Format:Unformatted list | Settings

Please review and commit when possible.

thanks,

DT

Version:7.x-3.0-rc3» 7.x-3.x-dev
Category:bug» feature

I'm not sure if this is a task, views should resolve.
You can do a lot of rewriting at field level and i think you should know what you are doing and - in that case - rewrite fields to your needs.

What do you think?

Category:feature» bug

@rokr, this is a bug fix for an existing feature in views.

The ability to "Add row class" is already in the style settings form, but it's broken.

This patch simply fixes the error to actually sanitize the string for output as one or more valid CSS classes.

Reassigning to bug report on that basis.

David, i checked that out. Thanks for correcting me.
I applied your patch and it works as expected for e.g. taxonomy terms consisting of two words.

Thank you, cheers, Ronald

I can also confirm that the patch applies cleanly and works as expected with the current dev release.

Status:Needs review» Reviewed & tested by the community

I agree with the RTBC.

Version:7.x-3.x-dev» 6.x-3.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

WoW a RTBC by multiple people, that's really uncommon :)
Thanks for providing the patch!

Committed to 7.x-3.x, though the port to 6.x-3.x need some work.

Version:6.x-3.x-dev» 7.x-3.x-dev
Status:Patch (to be ported)» Needs work

Okay i reverted this patch as it caused a lot of troubles #1448400: Row classes not showing since update

Hmm sorry to hear that, I wonder what needs adjusting?

Has been working fine from this corner..

update: am tracking the related issue.

3.2 release was working fine here too and I wasn't seeing any issues reported in #1448400: Row classes not showing since update.

I would just like to point out that transliteration should also be brought into the css-friendly string sanitation process.

I ran into this when trying to assign a css class based on a taxonomy term in Spanish, the character 'é' went right in to the css class, untouched, unharmed ;)

Any update on this issue?
I have tested the patch in #1 and it seems to fix row class issue correctly, without any of the problems described in #1448400: Row classes not showing since update.

It's a pity that this patch causes side-effects. The approach of this patch seems the right one to me.

Is there a possibility to add this patch again? Maybe in a two stage procedure? Deprecate the old behavior of automatically adding spaces, e.g. in between of text[tag].

In a second stage, a release later, recommit this patch again. Maybe we could trim whitespace in order to address the rationale behind the current usage of text[tag].

I can see why the issue with

clearfix[tl_main_contact]

becoming

clearfix-main-contact

occured as per http://drupal.org/node/1448400#comment-5635180

and that can be fixed by adding a space between the classes

clearfix [tl_main_contact]

http://drupal.org/node/1448400#comment-5636372
http://drupal.org/node/1448400#comment-5637808

But I'm not sure why no row class would show at all, as I'm unable to replicate the problem with no row class showing.

Once the cause of that issue is identified, I can roll a new patch.

DT

P.S Update: I have a theory, new patch pending.

Status:Needs work» Needs review
StatusFileSize
new1.39 KB
PASSED: [[SimpleTest]]: [MySQL] 1,472 pass(es).
[ View ]

OK, I see what the issue was there.

The flaw in the patch was that it was only applying the row class for view displays that used fields.

if ($this->uses_fields() && $this->view->field) { ...

So users with 'content' type view displays would have lost their row classes.

The attached updated patch fixes the issue and applies to both field and normal 'content' style view displays.

Token substitutions are applied if the view display uses fields.

As per eduardogz's request in #14, it also applies transliteration to the classes, if the transliteration module is enabled.

Please review and commit when possible.

Thanks!

DT

Patch in #16 works for me. Thank you!

what about this task? I see that this patch is not added into views module. Why?

+++ b/plugins/views_plugin_style.incundefined
@@ -122,16 +122,24 @@ class views_plugin_style extends views_plugin {
+        }else{
...
+          }          ¶
...
+        return implode(' ', $classes);

Couple of validations against drupal code style: some space, missing new line for else ...

+++ b/plugins/views_plugin_style.incundefined
@@ -122,16 +122,24 @@ class views_plugin_style extends views_plugin {
+        if(module_exists('transliteration')){
+          $classes = array_map('transliteration_get', $classes);
+        }

Views really tries to not use other contrib module, can't we totally avoid that?

In general patches aren't just committed, but we need people to test it, people should have reviewed the actual code etc.

In general patches aren't just committed, but we need people to test it, people should have reviewed the actual code etc.

I understand you, and I'm know that in Drupal 8 it will be harder to put new patches into stable tree, but I think this patch is important and it would be great if it will be commited into stable tree.

I'm preparing new patch, cause author forgot about strtolower in class name.

StatusFileSize
new1.4 KB
FAILED: [[SimpleTest]]: [MySQL] 1,617 pass(es), 10 fail(s), and 0 exception(s).
[ View ]

Ok, I made a new patch with strtolower in class name. I hope that drupal code style is proper ( I use phpstorm with drupal addons ), if not I want to apologise for that ;-)

Status:Needs review» Needs work

The last submitted patch, views-add-strtolower-1360186-21.patch, failed testing.

hah, funny thing, I've only added strtolower in class name ( and remove white space ). I see that QA systems are much more better than almost year ago ;-p

Can anyone prepare better patch? I dont have time to make another one :(

StatusFileSize
new1.38 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in 1360186-24.patch.
[ View ]

here is another take on patch #21
but I am not really good with patches (it would be a miracle if it passed formatting tests)

I say RTBC, it works as advertised

@jaroslaw.kaminski
Well the problem is that views hasn't been converted to the new codestyle, because there have been more important things todo.

... this @transliteration snippet should probably still be removed.

is there any chance that one of our latest patch ( GiorgoSK or mine ) will be added into views module in near future? I'm always afraid to use patches, cause its big possibility that me or my collegues will forget about patch module in every update ;-)

@transliteration. By this document: http://www.w3.org/TR/CSS21/syndata.html#characters we can use nationals signs as class names ftp://www.columbia.edu/kermit/public_html/utf8-t1.html so we can remove transliteration snippet but in my opinion it would be not comfortable to use non converted names ;-) So make a decision please and I can create next patch without transliteration.

Status:Needs work» Needs review

Sending last patch to testbot.

Status:Needs review» Needs work

The last submitted patch, 1360186-24.patch, failed testing.

@transliteration. By this document: http://www.w3.org/TR/CSS21/syndata.html#characters we can use nationals signs as class names ftp://www.columbia.edu/kermit/public_html/utf8-t1.html so we can remove transliteration snippet but in my opinion it would be not comfortable to use non converted names ;-) So make a decision please and I can create next patch without transliteration.

Well, if the admin things to put any kind of characters in there, it's not the problem of views to take care of it, IMHO.

Status:Needs work» Needs review
StatusFileSize
new1.37 KB
FAILED: [[SimpleTest]]: [MySQL] 1,617 pass(es), 10 fail(s), and 0 exception(s).
[ View ]

Here's an updated version of my original patch from #16 that works with both field and content based view displays.

It also includes drupal_html_class which calls drupal_clean_css_identifier(drupal_strtolower($class));

Status:Needs review» Needs work

The last submitted patch, 1360186-views-row-class-cleansing-2.patch, failed testing.

Your patch failed testing on one test only:
"Take sure that a custom css class is added to the output."

Seems about right. Could you explain what changes it makes to css? And maybe include needed test changes to reflect that? Should be relatively easy, just apply the same transformation to what tests look for.

StatusFileSize
new2.43 KB
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]

Hoping this will pass the tests. Fingers crossed.

Status:Needs work» Needs review

Testbot superman jenga dragon-time.

confirming patch in #33 works

thanks!

Looks great, I hope this get into the module fast :)

Status:Needs review» Reviewed & tested by the community

Also tested and working perfectly. Can't wait to see this in.

PS: The patch also apply on stable version 7.x-3.5.

StatusFileSize
new5.72 KB
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]

New patch including class cleaning for "Style settings" in the field.

StatusFileSize
new6.75 KB
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]

New patch, I forgot to replace some 'element_classes' and 'element_label_classes' methods.

We are using this patch already on our production site.

Status:Reviewed & tested by the community» Needs work

Please don't change patches dramatically and leave it RTBC.

In general i don't think we should remove functions given that especially this functionality could be indeed used by people in contrib/custom development.

Status:Needs work» Needs review
StatusFileSize
new4.72 KB
FAILED: [[SimpleTest]]: [MySQL] 1,430 pass(es), 37 fail(s), and 6 exception(s).
[ View ]

Here it is...

Status:Needs review» Needs work

The last submitted patch, views.code_.1360186-41.patch, failed testing.

Status:Needs work» Needs review

#41: views.code_.1360186-41.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, views.code_.1360186-41.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.74 KB
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]

Fix patch.

StatusFileSize
new1.26 KB
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]

Here's a new solution fixing the two previous problems.

I'm just providing new tokens, transliterated and you can used them where you want.

The only thing is that it's using the module_exists() function to check if the 'transliteration' module exists.

StatusFileSize
new3.34 KB
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]

New patch using a hook as suggested by merlinofchaos on IRC.

The implementation of the hooks are in Views just to show how it works, it could be in another module of course.

+++ b/handlers/views_handler_field.inc
@@ -315,6 +315,12 @@ class views_handler_field extends views_handler {
+        if (sizeof(module_implements('views_field_token_alter')) > 0) {
@@ -796,6 +802,13 @@ class views_handler_field extends views_handler {
+      // Make sure at least one module implements our hook.
+      if (sizeof(module_implements('views_field_token_alter_info')) > 0) {
@@ -1422,6 +1435,12 @@ If you would like to have the characters \'[\' and \']\' please use the html ent
+    if (sizeof(module_implements('views_field_token_alter')) > 0) {

I prefer to use count() as it is more explicit.

StatusFileSize
new3.34 KB
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]

New patch using count() instead of sizeof().

Any update on this please ?