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

Media Query in CSS File is not closing when using several media_query controls #1787

Closed
kodeoagency opened this issue Feb 4, 2018 · 8 comments

Comments

@kodeoagency
Copy link

Hi.
When I have only one media_query control in my sections it works just fine, but as soon there are more than one, than the following ones in the output CSS don't have a closing bracket.

This is my test setup:

 'logo_size_desktop' => [
	'type' => 'slider',
	'choices' => ['min' => '50','max' => '500','step' => '1'],
	'default' => '500',
	'output' => [['element' => '#logo img','property' => 'width','units'=>'px','media_query'=>'@media (min-width: 1280px)']],
],
'logo_size_tablet' => [
	'type' => 'slider',
	'choices' => ['min' => '50','max' => '500','step' => '1'],
	'default' => '400',
	'output' => [['element' => '#logo img','property' => 'width','units'=>'px','media_query'=>'@media (max-width: 1279px)']],
],
'logo_size_phone' => [
	'type' => 'slider',
	'choices' => ['min' => '50','max' => '500','step' => '1'],
	'default' => '300',
	'output' => [['element' => '#logo img','property' => 'width','units'=>'px','media_query'=>'@media (max-width: 767px)']],
],

This is the CSS output

@media (min-width:1280px){#logo img{width:500px;}
}
@media (max-width:1279px){#logo img{width:400px;}
@media (max-width:767px){#logo img{width:300px;
@aristath
Copy link
Contributor

aristath commented Feb 4, 2018

I just tried this and I'm getting this CSS: @media (min-width: 1280px){#logo img{width:500px;}}@media (max-width: 1279px){#logo img{width:400px;}}@media (max-width: 767px){#logo img{width:300px;}}
Beatutified:

@media (min-width: 1280px) {
  #logo img {
    width: 500px;
  }
}

@media (max-width: 1279px) {
  #logo img {
    width: 400px;
  }
}

@media (max-width: 767px) {
  #logo img {
    width: 300px;
  }
}

so that looks ok to me...
Perhaps there's something else wrong but I can't replicate it using the code you provided.
Also please next time you post some code try to make it a bit more readable... The snippet you posted is obviously passed-on to a custom function which then adds the fields to the sections etc.
I had to first "translate" your code to something that would be usable on my end.
Your code was 21 lines, but in order to test, I had to convert it and make it 69 lines so that I add the config, a section, then your fields, and expand their formatting a bit so that I may actually see what goes on where.
This is what I ended up with, and with that code on a vanilla twentyseventeen theme it seems to be working fine:

Kirki::add_config( 'my_config', array(
	'capability'  => 'edit_theme_options',
	'option_type' => 'theme_mod',
) );

Kirki::add_section( 'my_section', array(
    'title'          => esc_attr__( 'My Section', 'textdomain' ),
) );

Kirki::add_field( 'my_config', [
	'settings' => 'logo_size_desktop',
	'type'     => 'slider',
	'choices'  => [
		'min'  => '50',
		'max'  => '500',
		'step' => '1'
	],
	'default'  => '500',
	'output'   => [
		[
			'element'     => '#logo img',
			'property'    => 'width',
			'units'       =>'px',
			'media_query' => '@media (min-width: 1280px)',
		]
	],
	'section'  => 'my_section',

] );

Kirki::add_field( 'my_config', [
	'settings' => 'logo_size_tablet',
	'type'     => 'slider',
	'choices'  => [
		'min'  => '50',
		'max'  => '500',
		'step' => '1'
	],
	'default'  => '400',
	'output'   => [
		[
			'element'     => '#logo img',
			'property'    => 'width',
			'units'       => 'px',
			'media_query' => '@media (max-width: 1279px)',
		]
	],
	'section'  => 'my_section',
] );

Kirki::add_field( 'my_config', [
	'settings' => 'logo_size_phone',
	'type'     => 'slider',
	'choices'  => [
		'min'  => '50',
		'max'  => '500',
		'step' => '1'
	],
	'default'  => '300',
	'output'   => [
		[
			'element'     => '#logo img',
			'property'    => 'width',
			'units'       => 'px',
			'media_query' => '@media (max-width: 767px)',
		]
	],
	'section'  => 'my_section',
] );

@kodeoagency
Copy link
Author

Hello Aristath,
maybe the issue is because of our customization. We will check this and reply soon.
Sorry for pasting the code in the wrong format. Next time I will do it right.

@kodeoagency
Copy link
Author

Hello Aristath,
we have found and fixed the issue in our customization.
I will now close this ticket. Thank you for responding.

@kodeoagency
Copy link
Author

kodeoagency commented Feb 7, 2018

Sorry to say, but I need to reopen the issue.

The problems appears ONLY when the output is saved in a CSS file.

Please add the following filter to the code above

add_filter('kirki/dynamic_css/method', function()
{ return 'file'; }
);

Then the code in the output CSS will look like this:

@media (min-width:1280px){#logo img{width:281px;}}
@media (max-width:1279px){#logo img{width:185px;}
@media (max-width:767px){#logo img{width:71px;

@kodeoagency kodeoagency reopened this Feb 7, 2018
@aristath
Copy link
Contributor

aristath commented Feb 8, 2018

I believe I found the culprit... Haven't tested anything yet, didn't have time to test today, but looking at the code I think it's this part here: https://github.com/aristath/kirki/blob/v3.0.25/modules/css/class-kirki-css-to-file.php#L117-L124
Try commenting-out those lines and see if that fixes anything.
If it does, I'll need to find a better way to minify the css a bit 👍

@kodeoagency
Copy link
Author

Hello Aristath,
I made a test. Commenting-out those lines has fixed the problem 👍
It would be nice if you could consider this in your next update.

@kodeoagency kodeoagency changed the title Media Query in CSS not closing when using several media_query controls Media Query in CSS File is not closing when using several media_query controls Feb 9, 2018
@aristath
Copy link
Contributor

Pushed this to the 3.0.26 version.
minimizing the CSS is not as important as a bugfix.

@kodeoagency
Copy link
Author

THANK YOU VERY MUCH!!

aristath added a commit that referenced this issue Mar 17, 2018
* develop: (73 commits)
  fixes #1730
  fixes #1830
  GDPR: Load webfont-loader locally
  Update fonts
  fixes #1834
  Apply WordPress Coding Standards
  Update kirki-helper-class.md
  see #1797
  cleanup unused vars
  See #1807
  Additional fix for #1809
  fixes #1828
  fixes #1808
  fix #1814
  fix #1797
  fixes #1809
  Update sortable.md
  fixes #1787
  update webfonts & grunt
  changelog
  ...

# Conflicts:
#	modules/postmessage/class-kirki-modules-postmessage.php
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

No branches or pull requests

2 participants