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

add body class filter #101

Closed
wants to merge 1 commit into from
Closed

add body class filter #101

wants to merge 1 commit into from

Conversation

diggy
Copy link
Contributor

@diggy diggy commented Feb 21, 2017

Note: I prefer lowercase, dashed body classes (e.g. pll-nl-be), so did not use sanitize_html_class() (which would result in e.g. pll-nl_BE)

@chesio
Copy link
Contributor

chesio commented Feb 21, 2017

Hi,

If you want to style your content based on the language, why not use lang attribute of html element? Instead of body.pll-nl-be you can use html[lang="nl-BE"] body. I don't think extra class on body element is necessary, it brings nothing new.

@diggy
Copy link
Contributor Author

diggy commented Feb 21, 2017

  • WP users are familiar with body classes and their power
  • WP itself uses body classes extensively
  • The body classes are filterable, thus providing flexibility

@chesio
Copy link
Contributor

chesio commented Feb 22, 2017

All true, but not really arguments for adding duplicate information to page body. Someone skilled enough to discover that there is a body class with page language can certainly add a simple filter to functions.php, if he finds out there is no such class:

add_filter('body_class', function (array $classes) {
    $classes[] = str_replace('_', '-', get_locale());
    return $classes;
});

Also, having this filter in Polylang means that as soon as you stop using it, you lose the extra class and any rules in your CSS tied to it stop working. Whereas the snippet above or the html[lang="nl-BE"] body selector both work even with other multilanguage plugins (or even without any multilanguage plugin at all).

@diggy
Copy link
Contributor Author

diggy commented Feb 23, 2017

Thank you for your opinion, but aren't you slightly overthinking this? It's just some body classes. There won't be any noticeable performance impact because of this change. Also, something does not have to be "new" to be useful. And fwiw, if you switch from Polylang to another multilingual plugin, I'm pretty sure you'll have some other, more severe headaches to deal with than changing a few CSS selectors in your stylesheet. I'm definitely "skilled enough", but still I'd like to see this functionality out of the box. Laziness, it's a good thing.

@chesio
Copy link
Contributor

chesio commented Feb 23, 2017

...aren't you slightly overthinking this?

No, I prefer (and encourage) generic solutions to problems, so there's less for me to remember (and for others too). Laziness is a good thing, as you say.

Anyway, I said what I had on my mind and ultimately it's up to @Chouby to decide.

@Chouby
Copy link
Contributor

Chouby commented Feb 27, 2017

I tend to agree with @chesio.

There are least 2 other ways to use language specific styles without Polylang dependency which, in my opinion are better to use than an extra class.

  1. The :lang pseudo selector is a standard CSS rule. See https://www.w3schools.com/cssref/sel_lang.asp
  2. If you have more than a few specific rules, you can use a stylesheet dedicated to one locale, for example en_US.css or fr_FR.css at the root of your theme directory. The stylesheet is automatically loaded by WP.

@Chouby Chouby closed this Feb 27, 2017
@diggy
Copy link
Contributor Author

diggy commented Feb 27, 2017

No sweat, wrapped it in a plugin: https://github.com/diggy/polylang-body-class

@diggy diggy deleted the add-body-class-filter branch February 27, 2017 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants