twbs / bootstrap

The most popular HTML, CSS, and JavaScript framework for developing responsive, mobile first projects on the web.
https://getbootstrap.com
MIT License
170.88k stars 78.88k forks source link

Bootstrap 4.0.0-alpha3 .tag class breaks WordPress #20542

Closed maimairel closed 8 years ago

maimairel commented 8 years ago

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 commented 8 years ago

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

maimairel commented 8 years ago

And that is a bad WP practice.

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

mdo commented 8 years ago

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 commented 8 years ago

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 https://github.com/twbs/bootstrap/issues/20542#issuecomment-241288247, or mute the thread https://github.com/notifications/unsubscribe-auth/ACCMSEdERwXN_7MK-GJshvEXloOH6qjoks5qiNuygaJpZM4Jom2V .

gilronc commented 8 years ago

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

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

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

19094

@guylepage3 @cvrebert

cyberwombat commented 8 years ago

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 commented 8 years ago

@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

https://github.com/twbs/bootstrap/issues/19094 @mdo @cvrebert

maimairel commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

.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 commented 8 years ago

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

vs.

2) Wordpress-Boostrap users

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 commented 8 years ago

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

gilronc commented 8 years ago

@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 commented 8 years ago

@Olivia101, 26% is not the majority.

diggy commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

guylepage3 commented 8 years ago

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

As referenced in https://github.com/twbs/bootstrap/issues/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 commented 8 years ago

Fix coming at you in #21020.

tiesont commented 8 years ago

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 commented 8 years ago

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

ghost commented 8 years ago

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