new blocks#6
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces new blocks for the theme – one for offer accordions and one for news listings, including their styles, scripts, AJAX functionality, and PHP integration.
- Added SCSS styles and JS logic for interactive offer accordions
- Added PHP functions for enqueuing assets and filtering block data for both offer accordions and news
- Established AJAX functionality for the news block and provided corresponding asset files
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| web/app/themes/osom-theme/blocks/offer-accordions/style.scss | New SCSS for offer accordions block styling |
| web/app/themes/osom-theme/blocks/offer-accordions/script.js | New JavaScript for offer accordions interactions and carousel initialization |
| web/app/themes/osom-theme/blocks/offer-accordions/functions.php | PHP functions for enqueuing offer accordions assets and processing ACF data |
| web/app/themes/osom-theme/blocks/offer-accordions/ajax.js | New (currently empty) file likely intended for AJAX functionality |
| web/app/themes/osom-theme/blocks/news/style.scss | New SCSS file for news block styling |
| web/app/themes/osom-theme/blocks/news/script.js | New (currently empty) file likely intended for news block JS functionality |
| web/app/themes/osom-theme/blocks/news/functions.php | PHP functions to enqueue news block assets and filter news data via Timber |
| web/app/themes/osom-theme/blocks/news/ajax.js | JS implementing AJAX functionality for loading more news posts |
Comments suppressed due to low confidence (2)
WalkthroughThis update introduces new functionality and styling for two custom WordPress blocks: "news" and "offer-accordions." It adds AJAX-driven loading for news posts, interactive accordions with carousels for offers, and associated SCSS styles. Scripts and styles are conditionally enqueued, and Timber context filters prepare block data, including image processing and AJAX integration. Changes
Sequence Diagram(s)AJAX News Loading FlowsequenceDiagram
participant User
participant Browser
participant newsAjax (JS)
participant Server (WordPress)
User->>Browser: Clicks "Load More" button
Browser->>newsAjax (JS): Button click event
newsAjax (JS)->>Server (WordPress): AJAX POST (offset, count)
Server (WordPress)-->>newsAjax (JS): Returns HTML with new posts and total count
newsAjax (JS)->>Browser: Appends new posts, updates UI, hides button if done
Offer Accordions Initialization and InteractionsequenceDiagram
participant User
participant Browser
participant OfferAccordions (JS)
User->>Browser: Loads page
Browser->>OfferAccordions (JS): DOMContentLoaded
OfferAccordions (JS)->>Browser: Initializes Swiper carousels
User->>Browser: Clicks accordion open/close
Browser->>OfferAccordions (JS): Accordion button event
OfferAccordions (JS)->>Browser: Toggles accordion state and animates height
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 phpcs (3.7.2)web/app/themes/osom-theme/blocks/news/functions.phpWarning: PHP Startup: Invalid date.timezone value 'UTC', using 'UTC' instead in Unknown on line 0 Fatal error: Uncaught Error: Class "Phar" not found in /usr/local/bin/phpcs:3 web/app/themes/osom-theme/blocks/offer-accordions/functions.phpWarning: PHP Startup: Invalid date.timezone value 'UTC', using 'UTC' instead in Unknown on line 0 Fatal error: Uncaught Error: Class "Phar" not found in /usr/local/bin/phpcs:3 ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (8)
web/app/themes/osom-theme/blocks/news/script.js (1)
1-2: Consider adding a comment or removing this empty file.This empty file serves as a placeholder but may cause confusion. If future functionality is planned, add a comment explaining the file's purpose. Otherwise, consider removing it to avoid clutter.
+// Placeholder for future news block JavaScript functionality +// Current functionality is handled by ajax.jsweb/app/themes/osom-theme/blocks/offer-accordions/ajax.js (1)
1-2: Consider adding a comment or removing this empty file.This empty file appears to be a placeholder for future AJAX functionality. Add a comment explaining its intended purpose or remove it if no immediate functionality is planned.
+// Placeholder for future offer-accordions AJAX functionality +// Similar to news/ajax.js patternweb/app/themes/osom-theme/blocks/offer-accordions/script.js (2)
16-16: Risk of selector collision with dynamic indices.The selector
[data-offer-accordion="${index + 1}"]:not(.swiper-initialize)could potentially match unintended elements if multiple instances of the script run or if the DOM structure changes.Consider using a more specific selector or adding validation:
- const carousel = new Swiper(`[data-offer-accordion="${index + 1}"]:not(.swiper-initialize)`, { + const carouselSelector = `[data-offer-accordion="${index + 1}"]:not(.swiper-initialized)`; + const carouselElement = document.querySelector(carouselSelector); + + if (!carouselElement) { + console.warn(`Carousel element not found for selector: ${carouselSelector}`); + return; + } + + const carousel = new Swiper(carouselSelector, {
74-77: Consider using DOMContentLoaded for better performance.Using
readystatechangewith a timeout is unnecessary whenDOMContentLoadedcan be used more efficiently.-document.addEventListener('readystatechange', () => { - if (document.readyState !== 'complete') return; - setTimeout(() => OfferAccordions.init(), 1); -}); +document.addEventListener('DOMContentLoaded', () => { + OfferAccordions.init(); +});web/app/themes/osom-theme/blocks/news/ajax.js (1)
65-65: Consider conditional initialization.Auto-initializing the module on import might not be desirable in all contexts. Consider making initialization explicit.
export default newsAjax; - newsAjax.init(); + // Auto-initialize if DOM is ready, otherwise caller should initialize manually + if (document.readyState === 'loading') { + document.addEventListener('DOMContentLoaded', () => newsAjax.init()); + } else { + newsAjax.init(); + }web/app/themes/osom-theme/blocks/offer-accordions/style.scss (2)
193-193: Complex calc() expression may cause performance issues.The carousel width calculation is complex and uses CSS custom properties that might not be supported in older browsers.
Consider adding a fallback and potentially simplifying:
- width: calc(100% + ((100vw - var(--scrollbar-width) - 1600px) / 2)); + width: calc(100% + ((100vw - var(--scrollbar-width, 0px) - 1600px) / 2));Also consider adding a simpler fallback for older browsers:
+ width: calc(100% + 63px); /* Fallback for older browsers */ width: calc(100% + ((100vw - var(--scrollbar-width, 0px) - 1600px) / 2));
206-210: WordPress admin extension is redundant.The
@extend .offer-accordions;in the admin context is redundant since it's extending the same class.body.wp-admin { .offer-accordions { - @extend .offer-accordions; + // WordPress admin specific overrides can be added here if needed } }If no admin-specific styles are needed, consider removing this block entirely.
web/app/themes/osom-theme/blocks/news/style.scss (1)
36-62: Consider improving flexbox layout efficiency.The content section uses flexbox effectively, but the
min-height: 140pxon tablet might cause layout inconsistencies if content varies significantly.Consider using
align-items: stretchon the parent container to ensure consistent heights across news tiles:&__content { display: flex; flex-direction: column; + height: 100%; @include media($tablet, 'min') { - min-height: 140px; + justify-content: space-between; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
web/app/themes/osom-theme/blocks/news/ajax.js(1 hunks)web/app/themes/osom-theme/blocks/news/functions.php(1 hunks)web/app/themes/osom-theme/blocks/news/script.js(1 hunks)web/app/themes/osom-theme/blocks/news/style.scss(1 hunks)web/app/themes/osom-theme/blocks/offer-accordions/ajax.js(1 hunks)web/app/themes/osom-theme/blocks/offer-accordions/functions.php(1 hunks)web/app/themes/osom-theme/blocks/offer-accordions/script.js(1 hunks)web/app/themes/osom-theme/blocks/offer-accordions/style.scss(1 hunks)
🔇 Additional comments (3)
web/app/themes/osom-theme/blocks/offer-accordions/style.scss (1)
1-3: LGTM on imports and overall structure.The SCSS file is well-structured with proper imports of base styles, variables, and mixins. The responsive design approach using media queries is implemented correctly.
web/app/themes/osom-theme/blocks/news/style.scss (2)
1-3: LGTM! Clean import structure.The imports follow a logical order: variables, mixins, then typography. This ensures proper dependency resolution.
5-11: Good responsive margin implementation.The progressive enhancement approach from mobile-first to tablet breakpoint is well-implemented.
| const bodyHeight = body.getBoundingClientRect().height; | ||
| body.style.height = '0px'; | ||
| body.style.overflow = 'hidden'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential issue with height calculation timing.
The getBoundingClientRect().height call might return 0 if the element is not fully rendered or visible when this code executes. This could cause the accordion to not work properly.
Consider calculating the height when needed or adding a fallback:
- const bodyHeight = body.getBoundingClientRect().height;
- body.style.height = '0px';
- body.style.overflow = 'hidden';
+ let bodyHeight = 0;
+
+ // Calculate height on demand to ensure proper rendering
+ const calculateHeight = () => {
+ if (bodyHeight === 0) {
+ body.style.height = 'auto';
+ body.style.overflow = 'visible';
+ bodyHeight = body.getBoundingClientRect().height;
+ }
+ return bodyHeight;
+ };
+
+ body.style.height = '0px';
+ body.style.overflow = 'hidden';Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/app/themes/osom-theme/blocks/offer-accordions/script.js around lines 55
to 57, the height calculation using getBoundingClientRect().height may return 0
if the element is not fully rendered or visible at that moment. To fix this,
defer the height calculation until the element is fully rendered or visible, or
add a fallback height value to ensure the accordion functions correctly. This
can be done by triggering the calculation on an event when the element is
visible or using a timeout to delay the measurement.
| openButton.addEventListener('click', () => { | ||
| body.style.height = `${bodyHeight}px`; | ||
| setTimeout(() => body.style.overflow = 'unset', 200); | ||
| block.classList.add('offer-accordions__block--opened'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update height calculation in event listener.
With the height calculation refactor, update the open button event listener to use the dynamic height calculation:
openButton.addEventListener('click', () => {
- body.style.height = `${bodyHeight}px`;
+ body.style.height = `${calculateHeight()}px`;
setTimeout(() => body.style.overflow = 'unset', 200);
block.classList.add('offer-accordions__block--opened');
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| openButton.addEventListener('click', () => { | |
| body.style.height = `${bodyHeight}px`; | |
| setTimeout(() => body.style.overflow = 'unset', 200); | |
| block.classList.add('offer-accordions__block--opened'); | |
| }); | |
| openButton.addEventListener('click', () => { | |
| body.style.height = `${calculateHeight()}px`; | |
| setTimeout(() => body.style.overflow = 'unset', 200); | |
| block.classList.add('offer-accordions__block--opened'); | |
| }); |
🤖 Prompt for AI Agents
In web/app/themes/osom-theme/blocks/offer-accordions/script.js around lines 59
to 63, the open button event listener currently sets the body height using a
static variable bodyHeight. Update this to calculate the height dynamically
inside the event listener before setting body.style.height, ensuring it uses the
current height value at the time of the click.
| init() { | ||
| this.setButton(); | ||
|
|
||
| this.settings.initialPostsCount = document.querySelector('.news__tiles').children?.length; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check for DOM elements.
The code assumes .news__tiles exists but doesn't handle the case where it might not be found.
- this.settings.initialPostsCount = document.querySelector('.news__tiles').children?.length;
+ const tilesContainer = document.querySelector('.news__tiles');
+ this.settings.initialPostsCount = tilesContainer?.children?.length || 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.settings.initialPostsCount = document.querySelector('.news__tiles').children?.length; | |
| const tilesContainer = document.querySelector('.news__tiles'); | |
| this.settings.initialPostsCount = tilesContainer?.children?.length || 0; |
🤖 Prompt for AI Agents
In web/app/themes/osom-theme/blocks/news/ajax.js at line 9, the code assumes the
element with class '.news__tiles' exists without checking for null. Add a null
check to ensure the element is found before accessing its children property to
avoid runtime errors. If the element is not found, set initialPostsCount to 0 or
handle it appropriately.
| const submit = document.querySelector('.news .button'); | ||
|
|
||
| submit.addEventListener('click', () => this.getPosts()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check for submit button.
The code assumes the button exists but doesn't validate it before adding the event listener.
const submit = document.querySelector('.news .button');
-
- submit.addEventListener('click', () => this.getPosts());
+
+ if (submit) {
+ submit.addEventListener('click', () => this.getPosts());
+ } else {
+ console.warn('News submit button not found');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const submit = document.querySelector('.news .button'); | |
| submit.addEventListener('click', () => this.getPosts()); | |
| const submit = document.querySelector('.news .button'); | |
| if (submit) { | |
| submit.addEventListener('click', () => this.getPosts()); | |
| } else { | |
| console.warn('News submit button not found'); | |
| } |
🤖 Prompt for AI Agents
In web/app/themes/osom-theme/blocks/news/ajax.js around lines 12 to 14, the code
adds an event listener to the submit button without checking if the button
element actually exists, which can cause errors if the element is not found. Add
a null check to verify that the submit button is not null before calling
addEventListener on it to prevent runtime errors.
| buttonBehaviour(count) { | ||
| const getDisplaiedPosts = document.querySelector('.news__tiles').children; | ||
|
|
||
| if (count === getDisplaiedPosts.length) { | ||
| const submit = document.querySelector('.news .button'); | ||
| submit.classList.add('var_hide'); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Logic issue in button behavior.
The button hiding logic compares the server count with the displayed posts count, but this comparison might be flawed if the server returns a different count structure.
Add validation and improve the logic:
buttonBehaviour(count) {
- const getDisplaiedPosts = document.querySelector('.news__tiles').children;
+ const tilesContainer = document.querySelector('.news__tiles');
+
+ if (!tilesContainer) {
+ console.warn('News tiles container not found');
+ return;
+ }
+
+ const displayedPosts = tilesContainer.children;
- if (count === getDisplaiedPosts.length) {
+ if (count <= displayedPosts.length) {
const submit = document.querySelector('.news .button');
- submit.classList.add('var_hide');
+ if (submit) {
+ submit.classList.add('var_hide');
+ }
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| buttonBehaviour(count) { | |
| const getDisplaiedPosts = document.querySelector('.news__tiles').children; | |
| if (count === getDisplaiedPosts.length) { | |
| const submit = document.querySelector('.news .button'); | |
| submit.classList.add('var_hide'); | |
| } | |
| }, | |
| buttonBehaviour(count) { | |
| const tilesContainer = document.querySelector('.news__tiles'); | |
| if (!tilesContainer) { | |
| console.warn('News tiles container not found'); | |
| return; | |
| } | |
| const displayedPosts = tilesContainer.children; | |
| if (count <= displayedPosts.length) { | |
| const submit = document.querySelector('.news .button'); | |
| if (submit) { | |
| submit.classList.add('var_hide'); | |
| } | |
| } | |
| }, |
🤖 Prompt for AI Agents
In web/app/themes/osom-theme/blocks/news/ajax.js around lines 53 to 60, the
button hiding logic compares the server count directly with the number of
displayed posts, which may be incorrect if the server count structure differs.
To fix this, validate the server count input to ensure it is a number, and
adjust the comparison logic to accurately reflect when all posts are displayed,
possibly by comparing the count to the actual number of post elements or a
relevant property. Add checks to handle unexpected count formats gracefully
before deciding to hide the button.
| add_filter( | ||
| 'timber/acf-gutenberg-blocks-data/offer-accordions', | ||
| function ( $context ) { | ||
| $blocks = $context['fields']['blocks']; | ||
|
|
||
| if ( $blocks ) { | ||
| foreach ( $blocks as $key => $block ) { | ||
| $context['fields']['blocks'][ $key ]['image'] = wp_get_attachment_image( $block['image']['id'], array( 503, 335 ), false, array( 'class' => 'offer-accordions__image' ) ); | ||
|
|
||
| $images = $context['fields']['blocks'][ $key ]['images']; | ||
|
|
||
| if ( $images ) { | ||
| foreach ( $images as $index => $image ) { | ||
| $context['fields']['blocks'][ $key ]['images'][ $index ]['image'] = wp_get_attachment_image( $image['image']['id'], array( 441, 330 ), false, array( 'class' => 'section-gallery__image section-gallery__image--offer' ) ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return $context; | ||
| } | ||
| ); |
There was a problem hiding this comment.
Add input validation and error handling.
The code directly accesses array keys without validation, which could cause PHP warnings or errors if the expected data structure is missing.
Add proper validation:
add_filter(
'timber/acf-gutenberg-blocks-data/offer-accordions',
function ( $context ) {
- $blocks = $context['fields']['blocks'];
+ $blocks = $context['fields']['blocks'] ?? array();
if ( $blocks ) {
foreach ( $blocks as $key => $block ) {
+ if ( empty( $block['image']['id'] ) ) {
+ continue;
+ }
$context['fields']['blocks'][ $key ]['image'] = wp_get_attachment_image( $block['image']['id'], array( 503, 335 ), false, array( 'class' => 'offer-accordions__image' ) );
- $images = $context['fields']['blocks'][ $key ]['images'];
+ $images = $context['fields']['blocks'][ $key ]['images'] ?? array();
if ( $images ) {
foreach ( $images as $index => $image ) {
+ if ( empty( $image['image']['id'] ) ) {
+ continue;
+ }
$context['fields']['blocks'][ $key ]['images'][ $index ]['image'] = wp_get_attachment_image( $image['image']['id'], array( 441, 330 ), false, array( 'class' => 'section-gallery__image section-gallery__image--offer' ) );
}
}
}
}
return $context;
}
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| add_filter( | |
| 'timber/acf-gutenberg-blocks-data/offer-accordions', | |
| function ( $context ) { | |
| $blocks = $context['fields']['blocks']; | |
| if ( $blocks ) { | |
| foreach ( $blocks as $key => $block ) { | |
| $context['fields']['blocks'][ $key ]['image'] = wp_get_attachment_image( $block['image']['id'], array( 503, 335 ), false, array( 'class' => 'offer-accordions__image' ) ); | |
| $images = $context['fields']['blocks'][ $key ]['images']; | |
| if ( $images ) { | |
| foreach ( $images as $index => $image ) { | |
| $context['fields']['blocks'][ $key ]['images'][ $index ]['image'] = wp_get_attachment_image( $image['image']['id'], array( 441, 330 ), false, array( 'class' => 'section-gallery__image section-gallery__image--offer' ) ); | |
| } | |
| } | |
| } | |
| } | |
| return $context; | |
| } | |
| ); | |
| add_filter( | |
| 'timber/acf-gutenberg-blocks-data/offer-accordions', | |
| function ( $context ) { | |
| $blocks = $context['fields']['blocks'] ?? array(); | |
| if ( $blocks ) { | |
| foreach ( $blocks as $key => $block ) { | |
| if ( empty( $block['image']['id'] ) ) { | |
| continue; | |
| } | |
| $context['fields']['blocks'][ $key ]['image'] = wp_get_attachment_image( | |
| $block['image']['id'], | |
| array( 503, 335 ), | |
| false, | |
| array( 'class' => 'offer-accordions__image' ) | |
| ); | |
| $images = $context['fields']['blocks'][ $key ]['images'] ?? array(); | |
| if ( $images ) { | |
| foreach ( $images as $index => $image ) { | |
| if ( empty( $image['image']['id'] ) ) { | |
| continue; | |
| } | |
| $context['fields']['blocks'][ $key ]['images'][ $index ]['image'] = wp_get_attachment_image( | |
| $image['image']['id'], | |
| array( 441, 330 ), | |
| false, | |
| array( 'class' => 'section-gallery__image section-gallery__image--offer' ) | |
| ); | |
| } | |
| } | |
| } | |
| } | |
| return $context; | |
| } | |
| ); |
🤖 Prompt for AI Agents
In web/app/themes/osom-theme/blocks/offer-accordions/functions.php around lines
16 to 37, the code accesses array keys like 'blocks', 'image', and 'images'
without checking if they exist, which can cause PHP warnings or errors. Add
validation to ensure these keys exist and contain the expected data before
accessing them. Use functions like isset() or empty() to verify the presence and
validity of these keys and their nested elements before processing them.
| $offer_accordions_scripts = function () { | ||
| if ( has_block( 'acf/offer-accordions' ) ) { | ||
| $time = time(); | ||
| $theme_path = get_template_directory_uri(); | ||
|
|
||
| wp_enqueue_style( 'offer-accordions-css', $theme_path . '/dist/blocks/offer-accordions/style.css', array(), $time, 'all' ); | ||
| wp_enqueue_script( 'offer-accordions-js', $theme_path . '/dist/blocks/offer-accordions/script.js', array(), $time, true ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace time() with theme version for cache busting.
Using time() for cache busting causes assets to reload on every page load, negating browser caching benefits.
Use the theme version instead:
-$time = time();
+$version = wp_get_theme()->get('Version');
$theme_path = get_template_directory_uri();
-wp_enqueue_style( 'offer-accordions-css', $theme_path . '/dist/blocks/offer-accordions/style.css', array(), $time, 'all' );
-wp_enqueue_script( 'offer-accordions-js', $theme_path . '/dist/blocks/offer-accordions/script.js', array(), $time, true );
+wp_enqueue_style( 'offer-accordions-css', $theme_path . '/dist/blocks/offer-accordions/style.css', array(), $version, 'all' );
+wp_enqueue_script( 'offer-accordions-js', $theme_path . '/dist/blocks/offer-accordions/script.js', array(), $version, true );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $offer_accordions_scripts = function () { | |
| if ( has_block( 'acf/offer-accordions' ) ) { | |
| $time = time(); | |
| $theme_path = get_template_directory_uri(); | |
| wp_enqueue_style( 'offer-accordions-css', $theme_path . '/dist/blocks/offer-accordions/style.css', array(), $time, 'all' ); | |
| wp_enqueue_script( 'offer-accordions-js', $theme_path . '/dist/blocks/offer-accordions/script.js', array(), $time, true ); | |
| } | |
| }; | |
| $offer_accordions_scripts = function () { | |
| if ( has_block( 'acf/offer-accordions' ) ) { | |
| - $time = time(); | |
| + $version = wp_get_theme()->get( 'Version' ); | |
| $theme_path = get_template_directory_uri(); | |
| - wp_enqueue_style( 'offer-accordions-css', $theme_path . '/dist/blocks/offer-accordions/style.css', array(), $time, 'all' ); | |
| - wp_enqueue_script( 'offer-accordions-js', $theme_path . '/dist/blocks/offer-accordions/script.js', array(), $time, true ); | |
| + wp_enqueue_style( 'offer-accordions-css', $theme_path . '/dist/blocks/offer-accordions/style.css', array(), $version, 'all' ); | |
| + wp_enqueue_script( 'offer-accordions-js', $theme_path . '/dist/blocks/offer-accordions/script.js', array(), $version, true ); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In web/app/themes/osom-theme/blocks/offer-accordions/functions.php around lines
3 to 11, replace the use of time() for cache busting with the theme version.
Retrieve the theme version using wp_get_theme()->get('Version') and use it as
the version parameter in wp_enqueue_style and wp_enqueue_script calls to enable
effective browser caching.
| $block_name = 'news'; | ||
| $js_dir = get_template_directory_uri() . '/dist/blocks/' . $block_name . '/ajax.js'; | ||
| $ajax_action_name = $block_name; | ||
| $juniper_ajax = new \Juniper\Ajax\JuniperAjaxNews( $ajax_action_name, $js_dir, $block_name ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move AJAX instantiation inside action hook.
Global scope instantiation can cause issues if the class isn't available or if this code runs too early.
Wrap in an appropriate hook:
-$block_name = 'news';
-$js_dir = get_template_directory_uri() . '/dist/blocks/' . $block_name . '/ajax.js';
-$ajax_action_name = $block_name;
-$juniper_ajax = new \Juniper\Ajax\JuniperAjaxNews( $ajax_action_name, $js_dir, $block_name );
+add_action( 'init', function() {
+ $block_name = 'news';
+ $js_dir = get_template_directory_uri() . '/dist/blocks/' . $block_name . '/ajax.js';
+ $ajax_action_name = $block_name;
+ $juniper_ajax = new \Juniper\Ajax\JuniperAjaxNews( $ajax_action_name, $js_dir, $block_name );
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $block_name = 'news'; | |
| $js_dir = get_template_directory_uri() . '/dist/blocks/' . $block_name . '/ajax.js'; | |
| $ajax_action_name = $block_name; | |
| $juniper_ajax = new \Juniper\Ajax\JuniperAjaxNews( $ajax_action_name, $js_dir, $block_name ); | |
| add_action( 'init', function() { | |
| $block_name = 'news'; | |
| $js_dir = get_template_directory_uri() . '/dist/blocks/' . $block_name . '/ajax.js'; | |
| $ajax_action_name = $block_name; | |
| $juniper_ajax = new \Juniper\Ajax\JuniperAjaxNews( $ajax_action_name, $js_dir, $block_name ); | |
| }); |
🤖 Prompt for AI Agents
In web/app/themes/osom-theme/blocks/news/functions.php around lines 59 to 62,
the instantiation of the JuniperAjaxNews class is done in the global scope,
which can cause issues if the class is not yet loaded or if the code runs too
early. To fix this, move the instantiation inside a proper WordPress action hook
such as 'init' or 'wp_loaded' by wrapping the instantiation code in a function
and hooking that function to the chosen action to ensure the class is available
and the code runs at the correct time.
| add_action( | ||
| 'wp_enqueue_scripts', | ||
| function () { | ||
| if ( has_block( 'acf/news' ) ) { | ||
| $time = time(); | ||
| $theme_path = get_template_directory_uri(); | ||
|
|
||
| wp_enqueue_style( 'news-css', $theme_path . '/dist/blocks/news/style.css', array(), $time, 'all' ); | ||
| wp_enqueue_script( | ||
| 'news-js', | ||
| $theme_path . '/dist/blocks/news/script.js', | ||
| array(), | ||
| $time, | ||
| array( | ||
| 'strategy' => 'defer', | ||
| 'in_footer' => true, | ||
| ) | ||
| ); | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix cache busting and consider admin enqueue.
Same cache busting issue as the previous file, and missing admin enqueue for block editor.
Apply these improvements:
add_action(
'wp_enqueue_scripts',
function () {
if ( has_block( 'acf/news' ) ) {
- $time = time();
+ $version = wp_get_theme()->get('Version');
$theme_path = get_template_directory_uri();
- wp_enqueue_style( 'news-css', $theme_path . '/dist/blocks/news/style.css', array(), $time, 'all' );
+ wp_enqueue_style( 'news-css', $theme_path . '/dist/blocks/news/style.css', array(), $version, 'all' );
wp_enqueue_script(
'news-js',
$theme_path . '/dist/blocks/news/script.js',
array(),
- $time,
+ $version,
array(
'strategy' => 'defer',
'in_footer' => true,
)
);
}
}
);
+
+add_action( 'admin_enqueue_scripts', function() {
+ if ( has_block( 'acf/news' ) ) {
+ $version = wp_get_theme()->get('Version');
+ $theme_path = get_template_directory_uri();
+ wp_enqueue_style( 'news-css', $theme_path . '/dist/blocks/news/style.css', array(), $version, 'all' );
+ }
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| add_action( | |
| 'wp_enqueue_scripts', | |
| function () { | |
| if ( has_block( 'acf/news' ) ) { | |
| $time = time(); | |
| $theme_path = get_template_directory_uri(); | |
| wp_enqueue_style( 'news-css', $theme_path . '/dist/blocks/news/style.css', array(), $time, 'all' ); | |
| wp_enqueue_script( | |
| 'news-js', | |
| $theme_path . '/dist/blocks/news/script.js', | |
| array(), | |
| $time, | |
| array( | |
| 'strategy' => 'defer', | |
| 'in_footer' => true, | |
| ) | |
| ); | |
| } | |
| } | |
| ); | |
| add_action( | |
| 'wp_enqueue_scripts', | |
| function () { | |
| if ( has_block( 'acf/news' ) ) { | |
| $version = wp_get_theme()->get('Version'); | |
| $theme_path = get_template_directory_uri(); | |
| wp_enqueue_style( 'news-css', $theme_path . '/dist/blocks/news/style.css', array(), $version, 'all' ); | |
| wp_enqueue_script( | |
| 'news-js', | |
| $theme_path . '/dist/blocks/news/script.js', | |
| array(), | |
| $version, | |
| array( | |
| 'strategy' => 'defer', | |
| 'in_footer' => true, | |
| ) | |
| ); | |
| } | |
| } | |
| ); | |
| add_action( 'admin_enqueue_scripts', function() { | |
| if ( has_block( 'acf/news' ) ) { | |
| $version = wp_get_theme()->get('Version'); | |
| $theme_path = get_template_directory_uri(); | |
| wp_enqueue_style( 'news-css', $theme_path . '/dist/blocks/news/style.css', array(), $version, 'all' ); | |
| } | |
| } ); |
🤖 Prompt for AI Agents
In web/app/themes/osom-theme/blocks/news/functions.php lines 3 to 23, the cache
busting uses time() which changes on every request causing unnecessary reloads;
replace it with a file modification time-based version for stable cache busting.
Also, add enqueue logic for the block editor in the admin area using the
appropriate hook to ensure styles and scripts load correctly in the editor
context.
| add_filter( | ||
| 'timber/acf-gutenberg-blocks-data/news', | ||
| function ( $context ) { | ||
| $number_of_posts = $context['fields']['number_of_news']; | ||
|
|
||
| $args = array( | ||
| 'post_type' => 'news', | ||
| 'posts_per_page' => $number_of_posts, | ||
| 'post_status' => 'publish', | ||
| 'order' => 'DESC', | ||
| 'orderby' => 'date', | ||
| ); | ||
|
|
||
| $query = new WP_Query( $args ); | ||
|
|
||
| $posts = $query->posts; | ||
| $prepare_posts = array(); | ||
|
|
||
| foreach ( $posts as $post ) { | ||
| $prepare_posts[] = array( | ||
| 'title' => $post->post_title, | ||
| 'image' => wp_get_attachment_image( get_field( 'logo', $post->ID ), 'medium' ), | ||
| 'link' => get_field( 'news_link', $post->ID ), | ||
| 'post_date' => gmdate( 'F j.Y', strtotime( $post->post_date ) ), | ||
| ); | ||
| } | ||
|
|
||
| $context['news_count'] = $query->found_posts; | ||
| $context['fields']['news'] = $prepare_posts; | ||
|
|
||
| return $context; | ||
| } | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and optimize query.
The filter lacks input validation and the query could be optimized for better performance.
Add validation and optimize:
add_filter(
'timber/acf-gutenberg-blocks-data/news',
function ( $context ) {
- $number_of_posts = $context['fields']['number_of_news'];
+ $number_of_posts = absint( $context['fields']['number_of_news'] ?? 5 );
$args = array(
'post_type' => 'news',
'posts_per_page' => $number_of_posts,
'post_status' => 'publish',
'order' => 'DESC',
'orderby' => 'date',
+ 'fields' => 'ids', // Only get IDs for better performance
+ 'no_found_rows' => false, // We need found_posts
);
$query = new WP_Query( $args );
- $posts = $query->posts;
+ $post_ids = $query->posts;
$prepare_posts = array();
- foreach ( $posts as $post ) {
+ foreach ( $post_ids as $post_id ) {
+ $post_title = get_the_title( $post_id );
+ $logo_id = get_field( 'logo', $post_id );
+ $news_link = get_field( 'news_link', $post_id );
+ $post_date = get_the_date( 'F j.Y', $post_id );
+
$prepare_posts[] = array(
- 'title' => $post->post_title,
- 'image' => wp_get_attachment_image( get_field( 'logo', $post->ID ), 'medium' ),
- 'link' => get_field( 'news_link', $post->ID ),
- 'post_date' => gmdate( 'F j.Y', strtotime( $post->post_date ) ),
+ 'title' => esc_html( $post_title ),
+ 'image' => $logo_id ? wp_get_attachment_image( $logo_id, 'medium' ) : '',
+ 'link' => esc_url( $news_link ),
+ 'post_date' => $post_date,
);
}
$context['news_count'] = $query->found_posts;
$context['fields']['news'] = $prepare_posts;
return $context;
}
);🤖 Prompt for AI Agents
In web/app/themes/osom-theme/blocks/news/functions.php between lines 25 and 57,
add validation to ensure $context['fields']['number_of_news'] is a positive
integer before using it in the WP_Query arguments. Also, optimize the query by
adding 'no_found_rows' => true if pagination is not needed, and limit the fields
returned by specifying 'fields' => 'ids' if only IDs are required for further
processing. This will improve performance and prevent invalid input from causing
issues.
Summary by CodeRabbit
New Features
Enhancements
Style
Chores