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

Question about styles #908

Closed
Rayken opened this issue Apr 19, 2016 · 11 comments
Closed

Question about styles #908

Rayken opened this issue Apr 19, 2016 · 11 comments
Labels

Comments

@Rayken
Copy link

Rayken commented Apr 19, 2016

I'm on a specific use-case and I can't wrap my head around preventing the use of a lot of <div style="bunch-of-styles-from-get-theme-mod-here">. I'm new to Kirki and I love it thus far. It has tons of potential, but sometimes one has to ask in order to move forward.

I'm creating elements on the page using repeater fields. These elements gets a class based on a field inside it (could be generated from its title, another field, etc). Unfortunately I can't style this newly added element using output. I guess I could grab all the mods made, make a "stylesheet.php" and assign everything in there and enqueue that? Which could be one way of doing it.

I'm not sure using output is the method of choice here. I don't know any other way except inline styles or maybe a PHP-file generating styles. Point being every repeater field is a section on the page with its own background image or color and other various settings, so the question is how do I apply these styles?

Maybe there's a way I've yet to discover? I'd love some light on this issue!

@Rayken
Copy link
Author

Rayken commented Apr 19, 2016

Update: so I've been fiddeling around but I can't find a solution that I'm comfortable with. Adding inline styles with wp_add_inline_style() is a bit of a mess as it takes a "string", so formatting a string (or strings) with get_theme_mod stuff from array to string and so on is quite the thing.

Although trying to use a PHP-file to hold dynamic styles is no easy task either - not if you want to enqueue it and still access functions such as get_theme_mod(). I was even thinking of using admin-ajax.php and an action to "include" the file that way instead, but yeah I'm still a bit lost here.

@aristath
Copy link
Contributor

@Quaked I'm afraid I don't quite understand what you're trying to achieve...

I'm creating elements on the page using repeater fields.

ok

These elements gets a class based on a field inside it (could be generated from its title, another field, etc).

I think the key from the array could be nicely used to generate these classes...

Unfortunately I can't style this newly added element using output.

Definitely not!!!
Repeater don't work with the output argument... though possible, it would get messy and can't possibly be abstracted enough to suit all usecases (and therefore be included in Kirki out of the box).

I guess I could grab all the mods made, make a "stylesheet.php" and assign everything in there and enqueue that? Which could be one way of doing it.

You could, but from personal experience I would advise against it. This method has waaaay too many issues.

Adding inline styles with wp_add_inline_style() is a bit of a mess as it takes a "string", so formatting a string (or strings) with get_theme_mod stuff from array to string and so on is quite the thing.

Actually it's a lot simpler than any other implementation you can possibly think of...

I was even thinking of using admin-ajax.php and an action to "include" the file that way instead, but yeah I'm still a bit lost here.

It's doable and we have a similar implementation on https://github.com/aristath/kirki/blob/2.3.1/includes/styles/class-kirki-styles-frontend.php#L83-L86
The reason we don't advertise this is because a couple of months ago some users reported issues using the ajax method so we retired it pending more tests & optimization (work in progress).

I just added a kirki/styles_array filter that should help you add your inline styles: 17b5c1b

Example:

function my_custom_styles( $css ) {

    // $css['media-query']['element']['property'] = 'value';
    $css['global']['.div1']['width'] = '30%';
    $css['@media (max-width: 600px)']['.div1']['width'] = '50%';
    $css['@media (max-width: 600px)']['.div1']['max-width'] = '300px';
    $css['global']['.div2']['width'] = '15rem';

    return $css;

}
add_filter( 'kirki/css_array', 'my_custom_styles' );

Kirki will then take care of generating the actual CSS result from that array (also adding browser prefixes where needed).

Please keep in mind though that in order for the above to work, you must have some controls besides your repeater, using the output argument.
The above function will append the styles to the existing inline-css generated by Kirki, so if there are no inline styles (so no fields using output) nothing will happen.

I hope I understood correctly what you're trying to achieve...

Let me know if that helps!

@Rayken
Copy link
Author

Rayken commented Apr 19, 2016

Thanks for the reply! You're spot on. I went with the AJAX (much like the one you linked) even though some users have reported issues (what issues btw?). It gives me the cleanest approach to have one dynamic stylesheet to maintain and set up corresponding to the settings from Kirki. So far it's working as intended!

However I did add a nonce to the request.

@Rayken
Copy link
Author

Rayken commented Apr 19, 2016

@aristath Oh, I just noticed your comment:

I think the key from the array could be nicely used to generate these classes...

When I dump the get_theme_mod( 'my_repeater', false ) I have no unique ID or anything to separate one "row" from the other. Should I use a custom field inside and have a <input type="hidden"> and somehow make it unique so I can target this row specifically? I'm not following you on the 'key' approach, mind elaborating?

Right now I'm using "sanitize_title" on one of the text inputs from the repeater field, but this does not mean two rows cannot be named the same thus not being unique anymore. Any ideas?

@aristath
Copy link
Contributor

aristath commented Apr 21, 2016

When I dump the get_theme_mod( 'my_repeater', false ) I have no unique ID or anything to separate one "row" from the other.

Sure there is!
This is what I get when I var_dump a sample repeater field:

array (size=2)
  0 => 
    array (size=2)
      'link_text' => string 'Kirki Site' (length=10)
      'link_url' => string 'https://kirki.org' (length=17)
  1 => 
    array (size=2)
      'link_text' => string 'Kirki Repository' (length=16)
      'link_url' => string 'https://github.com/aristath/kirki' (length=33)

The 0 & 1 are the array keys there, so I can do this:

$value = get_theme_mod( 'my_setting' );
foreach ( $value as $key => $row ) {
    echo '<div class="my-div-' . intval( $key ) . '">';
        echo '<a href="' . esc_url( $row['link_url'] . '">' . esc_html( $row['link_text'] . '</a>';
    echo '</div>';
}

@Rayken
Copy link
Author

Rayken commented Apr 21, 2016

@aristath

Of course.. I was blindly neglecting the key of the array focusing on trying to get a unique identifier as a setting rather than looking at the whole array. Obviously this works as expected! Thank you.

On another note, I just wanted to update the "issue" here with a response to:

I just added a kirki/styles_array filter that should help you add your inline styles

So I previously stated I went with the "AJAX enqueued stylesheet.php" approach. While this works it gets enqueued too early in order to have a functioning preview pane (changes were in the customizer but not "stored" in get_theme_mod() so naturally it wasn't usable in the dynamic css file). So to have it work with the preview I had to create an additional wp_add_inline_style() to mimmick the changes (again) before the dynamic stylesheet output them. So now I had two places needing some love rather than one which was the reason I went with the approach in the first place.

What the new filter does is basically the same as having one wp_add_inline_style() that works both as "output" as well as in the preview pane. I went along with this approach and it just works. Thanks for adding it!

@Rayken
Copy link
Author

Rayken commented Apr 22, 2016

@aristath I could use some more information about this new filter to be honest. I'm using it as above but the styles I add are applied twice, inline.

First with: <style id='kirki-styles-global-inline-css' type='text/css'>the styles</style> and then again, specific to the theme it seems: <style id='kirki-styles-THEMENAME-inline-css' type='text/css'> same styles </style>

How can I ignore the "global inline css" and just apply my changes to the theme specific ones? Preferably using the filter but maybe with some argument or something? Right now I'm getting double the styles using add_filter( 'kirki/styles_array', array( get_class(), 'theme_custom_styles' ) ); maybe we could get a filter called theme_styles_array? Or am I doing something wrong? Thanks!

@Rayken
Copy link
Author

Rayken commented Apr 27, 2016

@aristath I've yet to find a solution to the above where using the filter 'kirki/styles_array' applies twice, once in 'global' and once in 'theme'. Got any suggestion how I can apply the custom styles to one of them? Preferably the theme inline style. Thanks!

@Rayken
Copy link
Author

Rayken commented Apr 28, 2016

@aristath Okay I did some more digging. I'm wrong in thinking it's a "theme" specific inline style, it's actually my custom Kirki configuration id. This is because it's looping all the configurations and adding inline styles for them as seen here: https://github.com/aristath/kirki/blob/develop/includes/styles/class-kirki-styles-frontend.php#L108

The issue remains however. Maybe there could be a filter such as
add_filter( 'kirki/styles_{config_id}_array', 'someFunc' );
so we can specifically target stuff per Kirki configuration?

As I've previously mentioned I'm getting all my settings added inline repeatedly, and if I were to have multiple configurations other than 'global' I'd end up with a lot of redundant <style> in my <head>.

It's perfectly fine to add styles per configuration, it even makes sense. But to have each configuration inline style having all styles added with kirk/styles_array does not. If you understand what I mean?

@aristath
Copy link
Contributor

sorry, I completely missed the previous posts 'cause this issue was marked as resolved.
Reopened and marked as a bug. I'll take a look at it later today 👍

@aristath aristath reopened this Apr 28, 2016
@aristath aristath added the bug label Apr 28, 2016
@aristath
Copy link
Contributor

If you pull the develop branch now there's a kirki/{$config_id}/styles filter you can use.

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

2 participants