Merged
Conversation
Owner
Author
|
@copilot can you review |
Contributor
Owner
Author
|
@gemini-cli /review |
|
🤖 Hi @fellyph, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review Summary
This pull request introduces new features and enhancements to the scroll animation block, primarily adding support for parallax effects and expanding the available animation options. The changes are well-implemented and follow the existing code style.
🔍 General Feedback
- The introduction of new animation options and the parallax effect is a great addition to the block's capabilities.
- The use of CSS custom properties for parallax strength is a clean and effective approach.
- There is some code duplication that could be refactored to improve maintainability.
- The new utility script is a helpful addition for development.
| extraProps['data-parallax-strength'] = parallaxStrength; | ||
| extraProps.style = { | ||
| ...props.style, | ||
| '--parallax-strength': `${parallaxStrength}px`, |
There was a problem hiding this comment.
🟡 The logic for adding animation and parallax attributes to blocks is duplicated in withAnimationAttributes and withListExtraProps. This could lead to maintenance issues if the logic needs to be updated in the future. Consider refactoring this into a shared function to improve code reuse and maintainability.
A shared function could accept the block's attributes and existing props as arguments and return the new set of props. This function could then be called from both withAnimationAttributes and withListExtraProps, ensuring consistency and making future changes easier.
Suggested change
| '--parallax-strength': `${parallaxStrength}px`, | |
| if (animationType === 'none' && !parallaxEnabled) { | |
| return extraProps; | |
| } | |
| if (animationType !== 'none') { | |
| extraProps['data-scroll-anim'] = '1'; | |
| extraProps.className = classnames(extraProps.className, `scroll-anim-${animationType}`); | |
| if (animationRange && animationRange !== 'default') { | |
| extraProps['data-anim-range'] = animationRange; | |
| } | |
| if (isInOutAnimation(animationType)) { | |
| extraProps.style = { | |
| ...extraProps.style, | |
| '--animation-entry-start': `${animationEntryStart}%`, | |
| '--animation-entry-end': `${animationEntryEnd}%`, | |
| '--animation-exit-start': `${animationExitStart}%`, | |
| '--animation-exit-end': `${animationExitEnd}%`, | |
| }; | |
| } else { | |
| extraProps.style = { | |
| ...extraProps.style, | |
| '--animation-entry-start': `${animationEntryStart}%`, | |
| '--animation-entry-end': `${animationEntryEnd}%`, | |
| }; | |
| } | |
| } | |
| if (parallaxEnabled) { | |
| extraProps['data-parallax'] = '1'; | |
| extraProps['data-parallax-strength'] = parallaxStrength; | |
| extraProps.style = { | |
| ...extraProps.style, | |
| '--parallax-strength': `${parallaxStrength}px`, | |
| }; | |
| } | |
| return extraProps; | |
| } | |
| ); | |
| /** | |
| * A filter that adds the animation attributes to the block's class name in the editor. | |
| * This is necessary for the animations to work in the editor. | |
| * | |
| * @param {Function} BlockListBlock The original block list block component. | |
| * @returns {Function} The modified block list block component. | |
| */ | |
| addFilter( | |
| 'editor.BlockListBlock', | |
| 'my-scroll-block/with-client-id', | |
| createHigherOrderComponent((BlockListBlock) => { | |
| return (props) => { | |
| if (!SUPPORTED_BLOCKS.includes(props.name)) { | |
| return <BlockListBlock {...props} />; | |
| } | |
| const { | |
| animationType = 'none', | |
| animationRange, | |
| animationEntryStart = 0, | |
| animationEntryEnd = 100, | |
| animationExitStart = 0, | |
| animationExitEnd = 100, | |
| parallaxEnabled = false, | |
| parallaxStrength = 50, | |
| } = props.attributes; | |
| const extraProps = {}; | |
| if (animationType !== 'none') { | |
| extraProps.className = classnames(props.className, `scroll-anim-${animationType}`); | |
| if (animationRange && animationRange !== 'default') { | |
| extraProps['data-anim-range'] = animationRange; | |
| } | |
| if (isInOutAnimation(animationType)) { | |
| extraProps.style = { | |
| ...props.style, | |
| '--animation-entry-start': `${animationEntryStart}%`, | |
| '--animation-entry-end': `${animationEntryEnd}%`, | |
| '--animation-exit-start': `${animationExitStart}%`, | |
| '--animation-exit-end': `${animationExitEnd}%`, | |
| }; | |
| } else { | |
| extraProps.style = { | |
| ...props.style, | |
| '--animation-entry-start': `${animationEntryStart}%`, | |
| '--animation-entry-end': `${animationEntryEnd}%`, | |
| }; | |
| } | |
| } | |
| if (parallaxEnabled) { | |
| extraProps['data-parallax'] = '1'; | |
| extraProps['data-parallax-strength'] = parallaxStrength; | |
| extraProps.style = { | |
| ...props.style, | |
| '--parallax-strength': `${parallaxStrength}px`, | |
| }; | |
| } | |
| return <BlockListBlock {...props} {...extraProps} />; | |
| }; | |
| }, 'withListExtraProps') | |
| ); | |
| /** |
| # Step 3: Escape " to \" for JSON | ||
| json_content = php_content.replace('"', '\\"') | ||
|
|
||
| print(json_content) |
There was a problem hiding this comment.
🟢 This script is a good utility for generating the content string. For improving readability and robustness, you could consider using f-strings for formatting and the json module for escaping, which is safer and handles more edge cases.
Suggested change
| print(json_content) | |
| +import json | |
| + | |
| +content = r'''<!-- wp:paragraph --> | |
| +<p>A trip to Portugal is an immersion into a land where history, culture, and nature blend seamlessly. From the sun-drenched beaches of the Algarve to the rugged volcanic landscapes of the Azores, Portugal offers a stunning diversity of scenery.</p> | |
| +<!-- /wp:paragraph --> | |
| + | |
| +<!-- wp:paragraph --> | |
| +<p>Wander through Lisbon's historic, tiled streets, where traditional fado music echoes from old neighborhoods. Or explore Porto, the capital of the Douro Valley, the world's oldest wine region.</p> | |
| +<!-- /wp:paragraph --> | |
| + | |
| +<!-- wp:image {"sizeSlug":"large","linkDestination":"none","className":"scroll-anim-block scroll-anim-scale-up"} --> | |
| +<figure class="wp-block-image size-large scroll-anim-block scroll-anim-scale-up"><img src="https://pub-4517acecab6543f0bc62af2fea95f2b6.r2.dev/cover-image.webp" alt=""/></figure> | |
| +<!-- /wp:image --> | |
| + | |
| +<!-- wp:paragraph --> | |
| +<p>Wander through Lisbon's historic, tiled streets, where traditional fado music echoes from old neighborhoods. Or explore Porto, the capital of the Douro Valley, the world's oldest wine region.</p> | |
| +<!-- /wp:paragraph --> | |
| + | |
| +<!-- wp:heading {"className":"scroll-anim-scale-up scroll-anim-block scroll-anim-fade-in"} --> | |
| +<h2 class="wp-block-heading scroll-anim-scale-up scroll-anim-block scroll-anim-fade-in">Cities of Portugal</h2> | |
| +<!-- /wp:heading --> | |
| + | |
| +<!-- wp:paragraph --> | |
| +<p>Wander through Lisbon's historic, tiled streets, where traditional fado music echoes from old neighborhoods. Or explore Porto, the capital of the Douro Valley, the world's oldest wine region.</p> | |
| +<!-- /wp:paragraph --> | |
| + | |
| +<!-- wp:columns {"align":"wide"} --> | |
| +<div class="wp-block-columns alignwide"><!-- wp:column --> | |
| +<div class="wp-block-column"><!-- wp:image {"sizeSlug":"large","linkDestination":"none","className":"scroll-anim-block scroll-anim-slide-in-right"} --> | |
| +<figure class="wp-block-image size-large scroll-anim-block scroll-anim-slide-in-right"><img src="https://pub-4517acecab6543f0bc62af2fea95f2b6.r2.dev/Gemini_Generated_Image_we7iutwe7iutwe7i.webp" alt=""/></figure> | |
| +<!-- /wp:image --></div> | |
| +<!-- /wp:column --> | |
| + | |
| +<!-- wp:column --> | |
| +<div class="wp-block-column"><!-- wp:image {"aspectRatio":"1","scale":"cover","sizeSlug":"large","linkDestination":"none","className":"scroll-anim-block scroll-anim-slide-in-up"} --> | |
| +<figure class="wp-block-image size-large scroll-anim-block scroll-anim-slide-in-up"><img src="https://pub-4517acecab6543f0bc62af2fea95f2b6.r2.dev/Gemini_Generated_Image_83jl2g83jl2g83jl.webp" alt="" style="aspect-ratio:1;object-fit:cover"/></figure> | |
| +<!-- /wp:image --></div> | |
| +<!-- /wp:column --> | |
| + | |
| +<!-- wp:column --> | |
| +<div class="wp-block-column"><!-- wp:image {"sizeSlug":"large","linkDestination":"none","className":"scroll-anim-block scroll-anim-slide-in-left"} --> | |
| +<figure class="wp-block-image size-large scroll-anim-block scroll-anim-slide-in-left"><img src="https://pub-4517acecab6543f0bc62af2fea95f2b6.r2.dev/Gemini_Generated_Image_pi34y8pi34y8pi34.webp" alt=""/></figure> | |
| +<!-- /wp:image --></div> | |
| +<!-- /wp:column --></div> | |
| +<!-- /wp:columns --> | |
| + | |
| +<!-- wp:columns {"align":"wide"} --> | |
| +<div class="wp-block-columns alignwide"><!-- wp:column --> | |
| +<div class="wp-block-column"><!-- wp:image {"sizeSlug":"large","linkDestination":"none","className":"scroll-anim-block scroll-anim-slide-in-right"} --> | |
| +<figure class="wp-block-image size-large scroll-anim-block scroll-anim-slide-in-right"><img src="https://pub-4517acecab6543f0bc62af2fea95f2b6.r2.dev/Gemini_Generated_Image_trof06trof06trof.webp" alt=""/></figure> | |
| +<!-- /wp:image --></div> | |
| +<!-- /wp:column --> | |
| + | |
| +<!-- wp:column --> | |
| +<div class="wp-block-column"><!-- wp:image {"aspectRatio":"1","scale":"cover","sizeSlug":"large","linkDestination":"none","className":"scroll-anim-slide-in-up scroll-anim-block scroll-anim-slide-in-left"} --> | |
| +<figure class="wp-block-image size-large scroll-anim-slide-in-up scroll-anim-block scroll-anim-slide-in-left"><img src="https://pub-4517acecab6543f0bc62af2fea95f2b6.r2.dev/Gemini_Generated_Image_we7iutwe7iutwe7i.webp" alt="" style="aspect-ratio:1;object-fit:cover"/></figure> | |
| +<!-- /wp:image --></div> | |
| +<!-- /wp:column --></div> | |
| +<!-- /wp:columns --> | |
| + | |
| +<!-- wp:paragraph --> | |
| +<p>Wander through Lisbon's historic, tiled streets, where traditional fado music echoes from old neighborhoods. Or explore Porto, the capital of the Douro Valley, the world's oldest wine region.</p> | |
| +<!-- /wp:paragraph --> | |
| + | |
| +<!-- wp:paragraph --> | |
| +<p>Wander through Lisbon's historic, tiled streets, where traditional fado music echoes from old neighborhoods. Or explore Porto, the capital of the Douro Valley, the world's oldest wine region.</p> | |
| +<!-- /wp:paragraph -->''' | |
| + | |
| +# Step 1: Escape for PHP and flatten to single line | |
| +php_content = content.replace("'", "\\'").replace("\n", "") | |
| + | |
| +# Step 2: Escape for JSON | |
| +# Using json.dumps is safer as it handles various edge cases. | |
| +# We slice the result of dumps to remove the surrounding quotes it adds. | |
| +json_content = json.dumps(php_content)[1:-1] | |
| + | |
| + | |
| +print(json_content) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces new features and enhancements to the scroll animation block, primarily adding support for parallax effects and expanding the available animation options. It also includes updates for improved attribute handling and corresponding CSS changes to support the new animations and parallax effect.
New Features and Enhancements
parallaxEnabledandparallaxStrengthallow users to enable and configure a parallax scrolling effect for supported blocks. UI controls for these options have been added to the Inspector panel, and relevant data attributes and CSS variables are injected into the block markup when enabled. (src/index.js,src/style.css) [1] [2] [3] [4] [5] [6] [7] [8]src/index.js,src/style.css) [1] [2] [3] [4]Other Improvements
src/index.js) [1] [2]blueprint.json: changednametotitlein the meta section for better alignment with expected schema fields. (blueprint.json)generate_content.pyfor generating and escaping block content for use in PHP and JSON contexts. (generate_content.py)