Skip to content

Commit

Permalink
fix: add inline style instead of increasing css size
Browse files Browse the repository at this point in the history
  • Loading branch information
@girishpanchal30
girishpanchal30 committed Sep 3, 2024
1 parent 2a49c55 commit 9afa3f4
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 14 deletions.
13 changes: 0 additions & 13 deletions assets/scss/components/main/_gutenberg.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,3 @@ blockquote {
margin-right: calc(50% - 35vw);
}
}

.wp-lightbox-overlay {

figure {

img {
height: var(--wp--lightbox-image-height);
min-height: var(--wp--lightbox-image-height);
min-width: var(--wp--lightbox-image-width);
width: var(--wp--lightbox-image-width);
}
}
}
30 changes: 30 additions & 0 deletions inc/views/template_parts.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class Template_Parts extends Base_View {
public function init() {
add_action( 'wp_enqueue_scripts', array( $this, 'add_featured_post_style' ) );
add_action( 'wp_enqueue_scripts', array( $this, 'add_vertical_spacing_style' ) );
add_action( 'wp_enqueue_scripts', array( $this, 'add_lightbox_style' ) );
add_action( 'neve_do_featured_post', array( $this, 'render_featured_post' ) );
add_action( 'neve_blog_post_template_part_content', array( $this, 'render_post' ) );
add_filter( 'excerpt_more', array( $this, 'link_excerpt_more' ) );
Expand Down Expand Up @@ -552,4 +553,33 @@ public function get_ordered_components( $associative = false ) {

return json_decode( get_theme_mod( 'neve_post_content_ordering', wp_json_encode( $default_ordered_components ) ), $associative );
}

/**
* Add inline lightbox style if the post content includes an image block with lightbox.
*/
public function add_lightbox_style() {
global $post;
if ( ! has_block( 'core/image', $post ) ) {
return;
}
$blocks = parse_blocks( $post->post_content );
$blocks = array_filter(
$blocks,
function( $block ) {
return 'core/image' === $block['blockName'] && ! empty( $block['attrs']['lightbox']['enabled'] );
}
);
if ( empty( $blocks ) ) {
return;
}
$inline_style = '
.wp-lightbox-overlay figure img {
height: var(--wp--lightbox-image-height);
min-height: var(--wp--lightbox-image-height);
min-width: var(--wp--lightbox-image-width);
width: var(--wp--lightbox-image-width);
}
';
wp_add_inline_style( 'neve-style', Dynamic_Css::minify_css( $inline_style ) );
}
}
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"gzip": false,
"running": false,
"webpack": false,
"limit": "38.3 KB",
"limit": "38.1 KB",
"path": "style-main-new.min.css"
}
],
Expand Down

5 comments on commit 9afa3f4

@ineagu
Copy link

@ineagu ineagu commented on 9afa3f4 Sep 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this is a good enough reason @abaicus right? it kinds of defeat the purpose of that limit for me.

@selul
Copy link
Contributor

@selul selul commented on 9afa3f4 Sep 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ineagu what reason are you thinking about? he just reverted the limit to the previous value?

@ineagu
Copy link

@ineagu ineagu commented on 9afa3f4 Sep 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that finding ways to add more css inline to not going over the limit isn't a good reason, since is still css and inline is not always the best idea, which is what I got from the commit title " add inline style instead of increasing css size"

If this is the best solution that's fine.

@selul
Copy link
Contributor

@selul selul commented on 9afa3f4 Sep 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the style is loaded conditionally when lightbox is used inside the image tag, if this is a general enough use case we can add to the main style too.

@ineagu
Copy link

@ineagu ineagu commented on 9afa3f4 Sep 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding it conditionally make sense, my thought was that the reason for doing this should be that is the better option, not because we don't want to increase the limit.

As I understand this condition that was added, is executed at every page request in order to decide if those 4 lines of css are needed, which I am not sure if is more or less optimal on un-cached pages. This type of non-critical css that require interactions to be useful, I would assume it can be added in a file, loaded in the footer.

But I don't have enough understanding on this aspect, the reason that trigged my comment was the commit title and the reason.

Please sign in to comment.