Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bootstrap 4.0.0-alpha3 .tag class breaks WordPress #20542

Closed
maimairel opened this issue Aug 19, 2016 · 28 comments
Closed

Bootstrap 4.0.0-alpha3 .tag class breaks WordPress #20542

maimairel opened this issue Aug 19, 2016 · 28 comments

Comments

@maimairel
Copy link

When using the new .tag class in WordPress, it is conflicting with the post tag class names applied to a lot of WordPress elements like the body class, hentry etc.

It just won't work on WordPress.

Any work around on this?

Thanks!

@cyberwombat
Copy link

Probably the easiest way is to remove/rename the .tag class in WP. You can do so by using the body_class filter.

@maimairel
Copy link
Author

maimairel commented Aug 20, 2016

And that is a bad WP practice.

body_class(), post_class() and probably more than I know add the .tag-* class to elements.

@mdo
Copy link
Member

mdo commented Aug 21, 2016

It's been a long time since I worked with Wordpress—is there a list of used classes in the default theme(s) we should avoid?

@maimairel
Copy link
Author

Here's a good article with possible classes:
http://www.wpbeginner.com/wp-themes/default-wordpress-generated-css-cheat-sheet-for-beginners/

On Monday, 22 August 2016, Mark Otto notifications@github.com wrote:

It's been a long time since I worked with Wordpress—is there a list of
used classes in the default theme(s) we should avoid?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#20542 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACCMSEdERwXN_7MK-GJshvEXloOH6qjoks5qiNuygaJpZM4Jom2V
.

@mdo mdo added the css label Aug 23, 2016
@gilronc
Copy link

gilronc commented Aug 25, 2016

I loved the idea behind .tag but it would bring unnecessary breakage to wordpress projects if that is the case. Maybe the class can be changed to .bs-tag or simply left as .label despite the readability concerns with the html tag. Personally, i've never had any readability issues with .label classes and tags because they're typically used in different contexts.

<label for="username">Click me</label>
<input type="text" id="username">

<span class="label label-primary">Gymnasium</span>

#19094
@guylepage3
@cvrebert

@cyberwombat
Copy link

I think WP uses this only on the body tag (not 100% sure). Perhaps a rule to not have .tag apply to the body would address all issues?

.tag:not(body) { // bs tag css }

https://jsfiddle.net/cyberwombat/yg4s5txs/1/

@guylepage3
Copy link

@gilronc it appears that wordpress is using .tags and not .tag.

Furthermore, they are also recommending to add use the .tag class to include a secondary descriptor such as;

.tag-[name]

https://codex.wordpress.org/Function_Reference/post_class
https://wordpress.org/support/topic/adding-tagcategory-name-as-css-class-in-query

#19094
@mdo
@cvrebert

@maimairel
Copy link
Author

maimairel commented Aug 25, 2016

@cyberwombat Not only the body, but also post entries. What if someone has success as a post tag? Then an entry would be assigned the tag-success class which will make the background green.

@guylepage3 a snippet from WordPress 4.6 function get_body_class()

} elseif ( is_tag() ) {
    $tag = $wp_query->get_queried_object();
    $classes[] = 'tag';
    if ( isset( $tag->term_id ) ) {
        $tag_class = sanitize_html_class( $tag->slug, $tag->term_id );
        if ( is_numeric( $tag_class ) || ! trim( $tag_class, '-' ) ) {
            $tag_class = $tag->term_id;
        }

        $classes[] = 'tag-' . $tag_class;
        $classes[] = 'tag-' . $tag->term_id;
    }
} 

There it adds the .tag class to the body.

@cyberwombat
Copy link

@maimairel tag-success requires the tag class to be present. We can modify the css to be:

.tag.tag-success { ...  }

As WP only uses .tag in the body tag and we prevent .tag from affecting body as mentioned earlier.

https://jsfiddle.net/cyberwombat/yg4s5txs/2/

Does that address the WP use cases?

@gilronc
Copy link

gilronc commented Aug 26, 2016

.tag or .tags are pretty generic and may cause conflicts......i really like .label. Either one will work though. My main development platform is Drupal so i won't really encounter any conflicts with Bootstrap's css. I'm using Bootstrap 4 and 3 currently to develop a Drupal 8 theme. Other frameworks typically use prefixes to solve the problem eg: bs-tag instead of .tag. The problem with prefixes is that it can cause code bloat pretty quickly, imagine prefixing cards, tables, alerts etc( .bs-cards, .bs-tables, .bs-alerts ).

Maybe the wordpress conflicts can be resolved quickly by themers who are capable of sub-theming the core wordpress themes and adding their own classes or removing the defaults.(Something that is always done anyways, from my experience). One thing that makes Bootstrap easy to use is that it is plug and play(Insert a stylesheet, add a couple classes and you're done)

Salesforce Lightening Framework, Almost everything is prefixed(It increases compatibility but I don't like seeing slds scattered on every html tag, it gets messy pretty quickly)
<div class="slds-col--padded slds-size--1-of-2 slds-medium-size--1-of-6 slds-large-size--4-of-12">3</div>
https://www.lightningdesignsystem.com/components/utilities/grid/

@guylepage3
Copy link

I feel I have to highly disagree on this instance @gilronc.

IMO, there are two fractions of Boostrap users in this instance.

1) ALL Bootstrap users

  • All Bootstrappers have to deal with the friction and troubles of the .label tag.

vs.

2) Wordpress-Boostrap users

  • Wordpress-Boostrap users are only a small fraction of Bootstrap users that have to deal with the Wordpress <body>/ .tag issue. Also there are a few solutions to the Wordpress-Boostrap users issue.

So having said that, I have to err on the side of the majority of Bootstrap users and push for the having the .tag tag implemented into v4 in replacement of the .label tag.

@cr101
Copy link

cr101 commented Aug 27, 2016

@guylepage3 let's keep it real. Currently 26% of all websites globally use WordPress so it's a massive fraction of Bootstrap users

@cvrebert cvrebert added the v4 label Aug 27, 2016
@gilronc
Copy link

gilronc commented Aug 27, 2016

@guylepage3 , .label or .tagis fine with me. I've just never really had any problems with the html <label> or a <span class="label"></span> causing me any concerns.

I do agree that it might cause friction and affect readability in some cases but .label has just been around forever ....Foundation, Salesforce and other micro-frameworks seems to have standardized on .labels , even github uses .label for their "tags" :)

Nothing is wrong with either one from my perspective, i'm fine with whichever one the core maintainers decide to go with.

@guylepage3
Copy link

@olivia101, 26% is not the majority.

@diggy
Copy link

diggy commented Aug 29, 2016

Here's a snippet to unset the tag body class:

add_filter( 'body_class', 'bs4_remove_tag_body_class' );
function bs4_remove_tag_body_class( $classes ) {
    if ( false !== ( $class = array_search( 'tag', $classes ) ) ) {
        unset( $classes[$class] );
    }
    return $classes;
}

There's no generic tag class added by the post_class() function, but adding WordPress post tags like "success" (generates tag-success class) etc. will cause trouble, w/ body as well as post class.

@eliottrobson
Copy link

@guylepage3 I believe he meant that was 26% of the internet, not Bootstrap users. So it could quite easily be a majority of Bootstrap users, it just depends on the overlap. I know I have seen A LOT of WP themes based on BS and would happily advocate for the framework to try and play nice with it :)

@guylepage3
Copy link

@eliottrobson thanks. Although I am still not convinced. We should find out what the actual number is then.
Also, there is a work around for it.
Lastly, IMO, I feel that software decisions should not be predicated nor effected by other, third party, software. Decisions should be made for the primary software.

@maimairel
Copy link
Author

I really still want to use the label component, but with it being .tag I have no choice than removing it from Bootstrap. It's too troublesome to adjust WordPress just to use something that can be coded easily ourself.
If Bootstrap 4 gets officially released with .tag, I'm pretty sure many WordPress users will encounter the same issue.

@guylepage3
Copy link

@maimairel, if you look at the workarounds presented above you'll notice that it's extremely easy to alter for your specific use case. The work arounds for all other Bootstrap users are definitely cumbersome and anti-semantic.

@bkaminski
Copy link

bkaminski commented Sep 12, 2016

@diggy your addition to functions.php did the trick for me.

WordPress has a popular feature that you add to the body tag to pull in WP classes based on the page you're on for the ability to edit those specific pages apart from other pages included in a Wordpress theme.

 <body <?php body_class(); ?>>

Translates to:

 <body class="archive tag tag-wordpress tag-165 logged-in">

Before workaround applied to functions.php

This attribute is calling in the class "tag" only on tag pages, I don't see this as a huge issue as a workaround has already been established and the "body class" is just empty class placeholders incase you want to edit a specific page class (IE: .tag, .home, .logged-in, etc).

But because WP generates what itself sees as an empty "tag" class, when BS4 is added, this becomes a naming-convention problem as it wants to apply the BS4 ".tag" class to the entire body.

None of these WP CSS classes are actually applying any rules to your page, they are there as utilities only incase you might want to.

Thanks for addressing this. I'm happy with a functions workaround, I was going to program some JS to do the same thing but this works!

Although, if a naming convention change is in consideration, might I offer up ".mark" or ".marker" or even ".bs-tag"? I can see .tag potentially running into similar issues with lesser used CMS's such as Drupal and Joomla, since the ability to "tag" posts/pages is kind of what made them what they are today.

@diggy
Copy link

diggy commented Sep 25, 2016

Improved snippet, removing all offending classes from WP's body and post class arrays:

add_filter( 'body_class', '_twbs_bootstrap_20542', 10, 1 );
add_filter( 'post_class', '_twbs_bootstrap_20542', 10, 1 );
function _twbs_bootstrap_20542( $classes )
{
    return array_diff( $classes, array(
        'tag',
        'tag-pill',
        'tag-default',
        'tag-info',
        'tag-warning',
        'tag-danger',
        'tag-success',
        'tag-primary',
    ) );
}

@kenashworth
Copy link

body.tag is a core and standard way of theming in WordPress. Removing it, because of a need to work around Bootstrap's .tag is worse then silly. I suppose we should just consider WordPress and Bootstrap 4 to be incompatible, officially? Let some users fork Bootstrap, some user break WordPress? This is a weird decision, resting on the "purity" of naming .tag.

@mdo
Copy link
Member

mdo commented Oct 19, 2016

We'll be getting the class name changed in Alpha 6.

@mdo mdo added this to the v4.0.0-alpha.6 milestone Oct 19, 2016
@guylepage3
Copy link

@mdo if we are going to change the class name.
May I request not going back to the old .label?

As referenced in #19094, there is a conflict with .label.

there is a naming conflict between bootstrap's class name .label and the html tag <label>. it is generally good practice to not name your classes after html tags for easy and clear recognition.

i'd like to propose that we change the bootstrap .label to something like .tag.

@mdo
Copy link
Member

mdo commented Oct 27, 2016

Fix coming at you in #21020.

@tiesont
Copy link
Contributor

tiesont commented Nov 2, 2016

I see that tag is changing back to badge in Alpha 6. Just as an FYI, I was also getting a conflict when using Bootstrap 4 with Prism.js - it uses the tag class for styling code that is either recognized or flagged as markup.

It's a roughly-equal amount of work to prefix either the generated class or the Bootstrap CSS, so this is mostly just a heads-up that there are other conflicts than WordPress.

Is it fairly likely that badge will be the "final" name for that component?

@tonystar
Copy link

tonystar commented Nov 2, 2016

@mdo why not just use btn-* modifier like btn btn-tag or even btn btn-pill?)

@ghost
Copy link

ghost commented Nov 2, 2016

Personally, I like the badge, class name, if we're trying to avoid any conflicts or misunderstandings. Just my subjective thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests