Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This is somewhat related to these issues:
#939992: "CSS classes must be alphanumeric or dashes only." I think underscores should be included
#1032380: More than one wrapper class
I would like CSS classes to be allowed to contain underscores, because those are heavily used in many popular CSS frameworks. Underscores get replaced by default in drupal_clean_css_identifier, unless an alternative filter is given as argument.
Comment | File | Size | Author |
---|---|---|---|
#66 | views-allow-double-underscores-1111258-66.patch | 599 bytes | lwalley |
| |||
#64 | views-allow-underscores-css-classes-1111258-64.patch | 1.72 KB | rodrigoaguilera |
| |||
#63 | 02.png | 11.8 KB | pvasili |
#63 | 01.png | 6.62 KB | pvasili |
#56 | views-allow-underscores-css-classes-1111258-55.patch | 2.11 KB | ChaseOnTheWeb |
Comments
Comment #1
Pisco CreditAttribution: Pisco commentedAdded a patch for review.
Comment #2
Pisco CreditAttribution: Pisco commentedRerolled patch versus current head.
Comment #3
dawehnerThanks! commited to 7.x-3.x
perhaps we should reintroduce the views specific function.
Comment #4
Pisco CreditAttribution: Pisco commentedI'd recommend that, I think it's weird having the custom filter
array('/' => '-', '[' => '-', ']' => '')
spread all over the code. In my opinion, the most beautiful and robust solution would be to fix drupal_clean_css_identifier: #1109854: Overly aggressive transliteration in drupal_clean_css_identifier removes underscores from classes. But that's another story I guess.Comment #5
dasjothe last commit causes the following site warnings for every row in my views:
reverted from the stated commit (a445bf4c80fe72c9c5ca09a5c9973b214c00ebc8) back to the previous (d00cc335086dd8533a2eee8401f9b8ece86a2c7c) and the warnings are gone.
Comment #6
dawehnerSo this needs work.
You can't add additional parameters if you use a callback.
Comment #7
dawehnerReverted the commit.
Comment #8
3rdLOF CreditAttribution: 3rdLOF commentedsubs.
Comment #9
BrightBoldThanks for the fix. Un-patched my install and all it well with the world.
Comment #10
3rdLOF CreditAttribution: 3rdLOF commentedSo wait....patch #2 is good to go?
Comment #11
Pisco CreditAttribution: Pisco commentedkannary100, no #2 is not good.
#6, yes you can, but not the way I expected :-( sorry for that!
I think we need a wrapper function for drupal_clean_css_identifier, someone mentioned there once was
views_css_safe
, or how about naming itviews_clean_css_identifier
?Where should I put this function?
Comment #12
merlinofchaos CreditAttribution: merlinofchaos commentedLet's restore views_css_safe() and in the doxygen for the function, include a notation of drupal_clean_css_identifier's problems.
THen let's use views_css_safe everywhere instead.
Comment #13
Jerome F CreditAttribution: Jerome F commentedSince I upgraded to views 7.x-3.x-dev datestamp = "1301660238"
running with drupal 7.x-dev release 2011-Mar-29 and ctools 7.x-1.0-alpha4
I have the following error message(s) :
Warning : strtr() [function.strtr]: The second argument is not an array in drupal_clean_css_identifier() (line 3630 dans (...)/public_html/includes/common.inc).
According to #5 this is related to this issue.
In addition the classes provided in the groups
<div>
and<ul>
are now blank in spite of the settings in List class and wrapper class - is that a related issue or may I submit a new one ?EDIT : I filed an issue there about that : http://drupal.org/node/1113482
Comment #14
3rdLOF CreditAttribution: 3rdLOF commented@Jerome F: That is the identical error I get..
Comment #15
dawehnerSo here is the new function.
Additonal you can use tokens now in more places.
Comment #16
dmsmidtsub
Comment #17
Jerome F CreditAttribution: Jerome F commented#15 Excellent patch ! We now can use tokens underscores and spaces for multiple classes in wrapper, label and
HTML element. Works for me all right.
EDIT : see next comment - my mistake
off topic : I love the new features and interface in views ! Great work.
Comment #18
Jerome F CreditAttribution: Jerome F commentedMy bad I spoke too soon - after saving and closing the view the following notices pops up :
It works ok in the views preview though.
Comment #19
Pisco CreditAttribution: Pisco commentedThanks dereine, I modified your patch slightly to avoid the errors mentioned by Jerome F.
Comment #20
Pisco CreditAttribution: Pisco commentedProperly formatted the patch.
Comment #21
dawehnerAs merlinofchaos pointed out row_index got lost somewhere for the other element_*_class functions.
Updated patch.
Comment #22
Jerome F CreditAttribution: Jerome F commentedPatch in #21 works for me.
Comment #23
Steven.Pescador CreditAttribution: Steven.Pescador commentedHi all, applied this patch but am still getting this error when viewing a Calendar...
Notice: Undefined property: view::$row_index in views_handler_field->get_render_tokens() (line 1187 of C:\wamp\www\my-site\sites\all\modules\views\handlers\views_handler_field.inc).
Apologize if this is the wrong place to post this, came here from http://drupal.org/node/1115222
Comment #24
dasjothe warning is gone so far.
in openlayers i get similar errors as SAF13 states in #23:
a list of
and
Comment #25
dawehner@dasjo, @saf13
This seems to be a bug of the openlayers module.
The default render function of style plugins set's the row_index
Additional commited the patch itself.
Comment #26
Steven.Pescador CreditAttribution: Steven.Pescador commented@dereine
Thanks for the feedback, as I don't have the openlayers module installed, am not sure where to go from here to sort out this notice..?
Comment #27
dawehnerWell the code in the openlayers module has to be fixed.
It has to set the $this->view->row_index for this.
Comment #28
dasjojust wanted to note that SAF13 stated another problem with calendar module in #23
Comment #29
Steven.Pescador CreditAttribution: Steven.Pescador commentedFound this thread last night and added the line of code, the notice has gone away...
http://drupal.org/node/1119752
Comment #30
gmopinillosv CreditAttribution: gmopinillosv commentedHello there
What does "Reverted the commit." mean?
Could you deliberate please.
Thank you
Comment #31
gmopinillosv CreditAttribution: gmopinillosv commentedHello there
Where I put that patch? I apologize I am new in this. Thank you
Comment #32
dawehnerBetter use the dev version, there is fixed a lot of stuff.
Comment #33
gmopinillosv CreditAttribution: gmopinillosv commentedThank dereine,
I used module "View version dev" and disappeared the error messages.
Thanks.
Comment #35
aaronsnoswell CreditAttribution: aaronsnoswell commentedThis is driving me mad - I'm using D7 and the 7.x-3.x-dev version of views (updated 2012-Mar-29) and underscores are still getting replaced with hyphens. I've tried applying the patch in #21, but it patch tells me it is already applied. That may be the case, but it's not having any affect.
Can anyone tell me how to stop drupal replacing underscores with hyphens for Views css classes?
Comment #36
aaronsnoswell CreditAttribution: aaronsnoswell commentedOK! After several hours of searching I found the one line fix that corrected this. Patch attached.
Comment #37
Jeordy CreditAttribution: Jeordy commentedsubscribing
Comment #38
gmclelland CreditAttribution: gmclelland commentedI just ran into this as well. This is big problem not just for grids, but if you also use the BEM approach for theming:
see http://nicolasgallagher.com/about-html-semantics-front-end-architecture/
see http://csswizardry.com/2013/01/mindbemding-getting-your-head-round-bem-s...
@aaronsnoswell - Your patch applied cleanly but didn't seem to work when I tried to apply a class on the view row.
I tried clearing caches, but nothing worked.
Step 1
Step 2
Step 3
Anybody have any other ideas?
Comment #39
carwin CreditAttribution: carwin commentedThis patch should do the trick:
Comment #40
Mołot CreditAttribution: Mołot commentedJust a quick question - why is it views request and not core request, if what basically bothers you is how core function works? And is there any reason views should deliberately work different than Drupal's core, generating different class names from the very same strings?
Comment #41
carwin CreditAttribution: carwin commentedThere actually is a core request for essentially the same thing, but due to the vast number of modules that depend on the way it works now, we're limited in what we can do in core. That's here: http://drupal.org/node/1109854.
Views already had an essentially duplicate function, I just started using it and updated the regex pattern.
I think the short answer is that Views is a bit more flexible on this subject than core is.
Comment #42
BrightBoldHmm. But if it's not likely to get changed in core, then is it unwise to put it in Views 7 since everyone's Views will break when they upgrade to Drupal 8 and Views is in core? (Oops, am I being a troublemaker?)
Comment #43
Mołot CreditAttribution: Mołot commented@BrightBold #42 - well, that was one of my 2 concerns. Second one is - I'm using the same data via views and outside views. Explaining to my designer why he needs to write same rules twice is irritating. One of the reasons to use Drupal was to avoid things like that. I know my opinion probably hardly matters, but "a bit more flexible" hardly seems reason enough to break core compatibility in something that will BE core in next release. Contrib module doing thing it's own way may be inconvenient, but is understandable. Parts of core doing same thing different ways should be no-go. Versions of the same project breaking on upgrade due to such a minor thing should be no-go too. Well, that's how I see it. But I'm open to others seeing things different. I would just like to know why they want what they want and why they think it's worth the risk and incompatibilities.
From my point of view, best thing would be to allow underscores in core. And in core or not, it should be configurable, with old way default on upgrade.
Comment #44
carwin CreditAttribution: carwin commented@Molot, I think I understand what you're saying here but I'm not positive.
It sounds like you're thinking of this in terms of 8.x / Views in core, but this issue specifically relates to the 7.x branch of Views, does that have any effect on your position?
Also, I don't know of any place where using
views_clean_css_identifier()
overdrupal_clean_css_identifier()
is going to break other modules. That function already existed in the code, it just wasn't being used. There's a good chance I missed a conversation about maybe deprecating that function since I'm not very familiar with Views' codebase – I may have missed something obvious about why this wasn't being used.I don't have a horse in this race, I'm just scratching an itch with this patch.
Comment #45
Mołot CreditAttribution: Mołot commented@carvin - "Versions of the same project breaking on upgrade due to such a minor thing should be no-go" that's my opinion about 7.x and 8.x and how should it matter that 7.x is not in core.
And I always thought that 2 functions / modules / APIs etc doing exactly the same thing is not a "Drupal way", that Drupal way is to have one tool for one task and cooperate, not compete. That's why many _clean_css_identifier() functions looks bad for me. Now, if I could be sure they give the same result for same input, or only one will be used by all modules in my project... I will have the same data rendered via views and our custom views plugins for readers, and more directly for management. They should share classes, for css readability if nothing else. But views extensions avoiding views-provided function? How can I be sure my co-developers will not use it? It would be natural to prefer views_* functions in views submodules, after all. And I bet I'm not alone with use-case like that.
Comment #46
carwin CreditAttribution: carwin commentedWhile I agree that having two similar functions isn't the best situation, I don't see how this specific patch is going to harm anything in Views by allowing you to throw an underscore in your class name. If/when you move your project over to d8 this shouldn't be a problem anymore because, as I understand it, d8 won't strip underscores. Drupal's CSS coding standards encourage the use of underscores in class names.
I don't think I grasp what you're getting at Mołot, so I'll just leave this up to others to review. If you want to remove the
views_clean_css_identifier()
function open a separate ticket about it.Comment #47
KrisBulman CreditAttribution: KrisBulman commented#39: views-allow-underscores-css-classes-1111258-39.patch queued for re-testing.
Comment #48
Harald Groven CreditAttribution: Harald Groven commented#39: views-allow-underscores-css-classes-1111258-39.patch queued for re-testing.
Comment #49
KrisBulman CreditAttribution: KrisBulman commentedJust wanted to point out that there is an RTBC core issue for 7.x that allows double underscores #2009584: Allow double underscores to pass through drupal_clean_css_identifier as per new CSS standards
Comment #50
drupov CreditAttribution: drupov commentedI have the same thing going as in #38.
Both the patches from here (#39) and from issue #2009584 (#16 there) worked.
My question: what would be the right one to choose?
Comment #51
DedMoroz CreditAttribution: DedMoroz commented#39 - works great.
Thanks!
Comment #52
emerham CreditAttribution: emerham as a volunteer commentedSo are we waiting for core to push a patch for this? It sounds like they have no intention on doing it yet for D7, but it made it into D8 core. If core isn't going to push it then I think views should do it.
Comment #53
carwin CreditAttribution: carwin at Lullabot commentedI agree, but I think this issue may be stuck in the bike shed.
Comment #54
ChaseOnTheWeb#39 breaks the ability to provide multiple space-separated classes. Patch attached.
(edit: Not sure why testbot failed to apply patch. It's working with drush make.)
Comment #56
ChaseOnTheWebRe-rolled against Views 3.14
Comment #57
ChaseOnTheWebComment #59
zalak.addweb CreditAttribution: zalak.addweb commentedComment #60
sphism CreditAttribution: sphism as a volunteer commentedWow... i just wasted an hour of my life because views was converting my nice BEM formatted class name underscores to hyphens
Comment #61
riddhi.addweb CreditAttribution: riddhi.addweb commentedComment #62
pvasili CreditAttribution: pvasili commentedAny progress in this error?
In php, you use your variable $my_var as $my_var.
Why in views $my_var variable I should use as $my-var :)??? LOL
Comment #63
pvasili CreditAttribution: pvasili commentedThe module panel works absolutely correctly in contrast views
Comment #64
rodrigoaguileraReroled agains latest dev
Comment #65
DamienMcKennaComment #66
lwalley CreditAttribution: lwalley commentedAn alternative patch that uses drupal_clean_css_identifier(), which now allows for double underscores, see #2009584: Allow double underscores to pass through drupal_clean_css_identifier as per new CSS standards.
Comment #67
Chris Matthews CreditAttribution: Chris Matthews as a volunteer commentedThe 2 year old patch in #64 does not apply to the latest views 7.x-3.x-dev, but the alternative patch in #66 applies cleanly and is probably the better approach.