stefanledin / responsify-wp

The WordPress plugin that cares about responsive images.
http://responsifywp.com
GNU General Public License v2.0
52 stars 8 forks source link

Suggestion: variable media-query #36

Open IvanPr opened 6 years ago

IvanPr commented 6 years ago

May I first thank you for a wonderful plugin, once again.

I would like to suggest another improvement toward the background-image suggestion.

Lets say I have following image-sets:

In the current plugin's logic flow the $media_queries has to look like: 'small-480' => 'min-width: 0', 'medium-768' => 'min-width: 480px', 'large-1024' => 'min-width: 768px', 'huge-1200' => 'min-width: 1024px'

This works great with the full-width background-images, BUT.... what if the plugin's rwp_style() is used to output an image for the banners of the grid [for example, 3 columns].... I would then like to set the $media_queries to look like: [full-width cells] 'small-480' => 'min-width: 0', 'medium-768' => 'min-width: 480px', [cells in 3-column grid] 'small-480' => 'min-width: 768px'

In this example, the grid cells would be full-width on mobile&tablets 0-480-768px BUT will be displayed in 3-column grid above 768px Screen Width.

The best image-size for the 3-column grid is 'small-480' once again, BUT... the plugin does not allow me to specify several 'media-queries' for the same image-size [example: small-48].

This is happening because the media_query() loop is going after existing '$image['media_query']['property']' instead of $image['media_query']['property'].

I would recommend to give users a freedom to choose as many $media_query sets as they want IF existing image-size is specified. Example:

[full-width cells on mobile and tablets] 'small-480' => 'min-width: 0', 'medium-768' => 'min-width: 480px', [3-column grid on desktops] 'small-480' => 'min-width: 768px', 'medium-768' => 'min-width: 1024px', [2 column cells on big displays] 'large-1024' => 'min-width: 1600px' [3-column grid on huge displays] 'medium-768' => 'min-width: 1900px'

Thank you.

IvanPr commented 6 years ago

I am talking about: /wp-content/plugins/responsify-wp/includes/style.php unction build_css() loop: for ($i=1; $i < count($this->images); $i++) { $media_query = $this->media_query($this->images[$i]); ... }

The function searches for the reference of a media-query for a specific existing image-set, instead of forming a loop of the assigned media-queries with the existing image-sets...

Here is the function itself: protected function build_css($selector) { if (empty($this->images[0])) return;

    $css = '<style>';
        $css .= $selector . ' {';
            $css .= 'background-image: url("'.$this->images[0]['src'].'");';
        $css .= '}';
        for ($i=1; $i < count($this->images); $i++) {
            $media_query = $this->media_query($this->images[$i]);
            $css .= $media_query.' {';
                $css .= $selector . '{';
                    $css .= 'background-image: url("'.$this->images[$i]['src'].'");';
                $css .= '}';
            $css .= '}';
        }
    $css .= '</style>';
    return $css;
}
stefanledin commented 6 years ago

Hi Ivan! Thanks for the suggestion, I think that I understand how you mean. How to you think the syntax should look like?

(Here's the example from the documentation)

<?php
echo rwp_style( $dynamic_header_image_ID, array(
  'selector' => '#hero',
  'sizes' => array('medium', 'large', 'full'),
  'media_queries' => array(
    'large' => 'min-width: 500px',
    'full' => 'min-width: 1024px'
  )
) );
?>
IvanPr commented 6 years ago

Stefan, the function syntax is perfect. I would not change drastically it in the new plugin's version, since there are people who already use it.

You've made a great choice selecting 'media_queries' to be an array. This will allow to add as many media-breakpoints and image-size switches as a developer need.

As an example, it would be great to generate the responsive background-image CSS for the grid-cells like this:

<?php
echo rwp_style( $dynamic_header_image_ID, array(
  'selector' => '#hero',
  'sizes' => array('small-480', 'medium-768', 'large-1024'), # this is not really needed with the new approach
  'media_queries' => array(
    'small-480' => 'min-width: 0px', # full-width cells on mobile and tablets
    'medium-768' => 'min-width: 480px', # still full-width, but a 480px wide image above 480px screens
    'small-480' => 'min-width: 768px', # switching to 3-column grid above 768px width
    'medium-768' => 'min-width: 1024px', # still 3-column grid but with bigger bg.image in a cell
  )
) );
?>

In the above example, an image-size could be used several times [it is not possible now] with as many media-switches as needed. The syntax stays the same.

Notice 'large-1024' in the 'sizes' => array() with the new approach it would not be used. I would actually make it like this:

The expected output would look like:

<style>
#hero {background-image: url("http://domain.com/images/small-480.jpg");}
@media screen and (min-width: 0px) {#hero{background-image: url("http://domain.com/images/small-480.jpg");}}
@media screen and (min-width: 480px) {#hero{background-image: url("http://domain.com/images/medium-768.jpg");}}
@media screen and (min-width: 768px) {#hero{background-image: url("http://domain.com/images/small-480.jpg");}}
@media screen and (min-width: 1024px) {#hero{background-image: url("http://domain.com/images/medium-768.jpg");}}
</style>

p.s. It would also be nice to be able to specify the selector with spaces, such as 'selector' => '#hero div.banner'. At the moment, your plugin would see the '#hero .banner' as '#hero' - the after-space part is cut out. I had to use something space-less like ''#hero>div.banner' in order to force the plugin to work as I want....

I hope, this explanation, helps. The suggested approach, takes the single-image-size-usage away and allows to use the plugin in the grids and in various in-page elements.

IvanPr commented 6 years ago

As a fact, if you call now:

<?php
echo rwp_style( $dynamic_header_image_ID, array(
  'selector' => '#hero',
  'sizes' => array('small-480', 'medium-768', 'large-1024'),
  'media_queries' => array(
    'small-480' => 'min-width: 0px',
    'medium-768' => 'min-width: 480px',
    'small-480' => 'min-width: 768px',
  )
) );
?>

The following generated rule will never be used:

@media screen and (min-width: 768px)
#hero {
    background-image: url(https://domain.com/images/small-480.jpg);
}

It will be overtaken by the following: @media screen and (min-width: 480px)

#hero {
    background-image: url(https://domain.com/images/medium-768.jpg);
}

This is a result of an image-size loop in the PHP function, instead of the media-size loop.