-
Notifications
You must be signed in to change notification settings - Fork 8
Add Marquee block - a8c timeline version #77
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
base: trunk
Are you sure you want to change the base?
Conversation
WalkthroughThis change introduces a new WordPress plugin named "Marquee" designed to provide a customizable marquee effect for content using the block editor. The update includes all foundational files and configuration for the plugin, such as Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
🔭 Outside diff range comments (2)
marquee/readme.txt (1)
9-56: Update readme content to reflect actual marquee functionalityThe readme.txt contains boilerplate content from the Create Block tool. For a production release, update:
- Line 9: Replace "Example block scaffolded with Create Block tool" with actual marquee block description
- Lines 13-16: Replace generic description with marquee-specific features and usage
- Lines 30-36: Replace placeholder FAQ with relevant marquee questions
- Lines 40-44: Update screenshot descriptions to reflect marquee functionality
-Example block scaffolded with Create Block tool. +A customizable marquee block that creates smooth scrolling text and content animations. == Description == -This is the long description. No limit, and you can use Markdown (as well as in the following sections). - -For backwards compatibility, if this section is missing, the full length of the short description will be used, and -Markdown parsed. +Create eye-catching scrolling content with the Marquee block. Features include: + +* Customizable scroll direction (left, right, up, down) +* Adjustable speed settings +* Pause on hover functionality +* Fade edges effect +* Responsive design +* Support for any inner block contentmarquee/marquee copy.php (1)
1-61: Remove duplicate plugin file or clarify its purpose.This file is nearly identical to
marquee.phpwith only a path difference in the block registration (line 58:/buildvs/build/marquee). Having duplicate plugin files can cause:
- Plugin conflicts if both are activated
- Maintenance confusion requiring changes in multiple places
- Inconsistent behavior due to path differences
Please clarify the purpose of this file and consider removing it if it's not needed.
If this is a development/testing version, consider:
- Moving it outside the main plugin directory
- Adding clear comments explaining its purpose
- Using a different approach like environment-based configuration
🧹 Nitpick comments (8)
marquee/classes/class-wpcomsp-blocks-self-update.php (2)
54-56: Fix comment mismatch with actual URLThe comment mentions "opsoasis.mystagingwebsite.com" but the actual URL is "opsoasis.wpspecialprojects.com".
- // Ask opsoasis.mystagingwebsite.com if there's an update. + // Ask opsoasis.wpspecialprojects.com if there's an update.
65-68: Fix comment mismatch and improve clarityThe comment references the wrong domain again.
- // Bail if this plugin wasn't found on opsoasis.mystagingwebsite.com. + // Bail if this plugin wasn't found on the update server.marquee/CHANGELOG.md (1)
1-2: Consider expanding changelog with feature detailsFor better documentation, consider adding more details about the initial release features and any known issues.
-0.1.0 -Initial release +## 0.1.0 +Initial release + +### Features +- Marquee block with customizable direction (left, right, up, down) +- Adjustable scroll speed +- Pause on hover functionality +- Fade edges effect +- Gap control for spacing + +### Known Issues +- Fade edges option may prevent scrolling in some casesmarquee/src/marquee/save.js (1)
18-36: Consider extracting className construction for better maintainabilityThe className construction could be more readable and maintainable.
export default function save({ attributes }) { const { direction, speed, pauseOnHover, gap, fadeEdges } = attributes; + + const classNames = [ + 'wp-block-a8csp-marquee', + `direction-${direction}`, + `speed-${speed}`, + fadeEdges && 'has-fade', + pauseOnHover && 'has-pause-on-hover' + ].filter(Boolean).join(' '); + const blockProps = useBlockProps.save({ - className: `wp-block-a8csp-marquee direction-${direction} speed-${speed}${fadeEdges ? ' has-fade' : ''}${pauseOnHover ? ' has-pause-on-hover' : ''}`, + className: classNames, style: { '--marquee-gap': `${gap}px` } });marquee/src/marquee/edit.js (1)
78-80: Consider adding speed unit indication.The speed range control doesn't indicate what the units represent, which could confuse users about the expected behavior.
Apply this diff to add help text:
<RangeControl label={__('Speed', 'marquee')} + help={__('Higher values = faster scrolling', 'marquee')} value={speed} onChange={(value) => setAttributes({ speed: value })} min={10} max={200} />marquee/src/marquee/editor.scss (1)
19-26: Excessive use of !important may cause maintenance issues.While the !important declarations ensure layout consistency, they make the styles harder to override and maintain. Consider using more specific selectors instead.
Consider this alternative approach with higher specificity:
- .marquee-items { - // Force horizontal layout - display: flex !important; - flex-direction: row !important; - flex-wrap: nowrap !important; - gap: var(--marquee-gap, 20px) !important; + .marquee-items, + .marquee-items.block-editor-block-list__layout { + display: flex; + flex-direction: row; + flex-wrap: nowrap; + gap: var(--marquee-gap, 20px); padding: 10px 0; - // Explicitly disable animation in editor - animation: none !important; + animation: none;marquee/src/marquee/style.scss (1)
27-30: Complex selector may cause specificity issues.The nested negation selector
:not(.block-editor-block-list__layout *)is complex and may cause maintenance challenges or unexpected behavior.Consider using a more explicit approach with CSS classes:
- // Only apply animation on frontend by excluding the block editor - &:not(.block-editor-block-list__layout *) { - animation: marquee var(--duration, 12s) linear infinite; - animation-play-state: var(--play-state, running); - } + // Animation controlled by body class or data attribute + body:not(.block-editor-page) & { + animation: marquee var(--duration, 12s) linear infinite; + animation-play-state: var(--play-state, running); + }marquee/src/marquee/view.js (1)
102-104: Unnecessary cleanup on beforeunload.The beforeunload event cleanup is unnecessary since the page is being unloaded anyway, and it could potentially interfere with the browser's cleanup process.
Consider removing this cleanup:
- // Cleanup on page leave - window.addEventListener('beforeunload', () => { - itemsContainer.style.setProperty('--play-state', 'running'); - });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
marquee/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
marquee/.editorconfig(1 hunks)marquee/.gitignore(1 hunks)marquee/CHANGELOG.md(1 hunks)marquee/classes/class-wpcomsp-blocks-self-update.php(1 hunks)marquee/marquee copy.php(1 hunks)marquee/marquee.php(1 hunks)marquee/package.json(1 hunks)marquee/readme.txt(1 hunks)marquee/src/marquee/block.json(1 hunks)marquee/src/marquee/edit.js(1 hunks)marquee/src/marquee/editor.scss(1 hunks)marquee/src/marquee/index.js(1 hunks)marquee/src/marquee/save.js(1 hunks)marquee/src/marquee/style.scss(1 hunks)marquee/src/marquee/view.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
marquee/src/marquee/save.js (2)
marquee/src/marquee/edit.js (2)
attributes(36-36)blockProps(43-50)marquee/src/marquee/view.js (2)
speed(39-39)pauseOnHover(48-48)
marquee/marquee.php (2)
marquee/classes/class-wpcomsp-blocks-self-update.php (3)
WPCOMSP_Blocks_Self_Update(13-80)get_instance(22-28)hooks(33-35)marquee/marquee copy.php (1)
a8csp_marquee_block_init(57-59)
marquee/src/marquee/edit.js (2)
marquee/src/marquee/save.js (2)
attributes(19-19)blockProps(20-25)marquee/src/marquee/view.js (3)
itemsContainer(32-32)speed(39-39)pauseOnHover(48-48)
marquee/src/marquee/index.js (1)
marquee/src/marquee/edit.js (1)
Edit(35-112)
🔇 Additional comments (23)
marquee/.editorconfig (1)
16-18: YAML override looks goodYAML files correctly switch to 2-space indentation, matching common tooling expectations.
marquee/.gitignore (1)
1-31: LGTM - Standard .gitignore configurationThe .gitignore file properly excludes common development artifacts, build outputs, and sensitive files. All patterns are appropriate for a WordPress plugin with Node.js build tooling.
marquee/src/marquee/save.js (2)
21-21: Fix className construction and spacing issuesThe className construction has spacing issues that could affect CSS targeting.
- className: `wp-block-a8csp-marquee direction-${direction}${fadeEdges ? ' has-fade' : ''} speed-${speed} ${pauseOnHover ? ' has-pause-on-hover' : ''}`, + className: `wp-block-a8csp-marquee direction-${direction} speed-${speed}${fadeEdges ? ' has-fade' : ''}${pauseOnHover ? ' has-pause-on-hover' : ''}`,The original code had:
- Missing space before
speed-${speed}- Extra space before the pauseOnHover conditional
This could cause CSS selectors to fail or create malformed class names.
Likely an incorrect or invalid review comment.
19-25: No CSS interference from.has-fademaskI’ve confirmed that the
.has-faderule inmarquee/src/marquee/style.scssonly applies a CSS mask (viamask-image/-webkit-mask-image) to the existing.marquee-contentcontainer. It doesn’t alter overflow, positioning, or animation properties, so it shouldn’t interrupt the marquee’s scrolling behavior.marquee/package.json (3)
1-7: LGTM!The package metadata is well-structured and follows WordPress block development standards. The version "0.1.0" appropriately reflects the WIP status mentioned in the PR objectives.
8-16: Scripts configuration supports development workflow.The npm scripts are well-configured for WordPress block development and include the linting scripts mentioned as needed in the PR objectives.
17-19: @wordpress/scripts is current at ^30.20.0The devDependency is already using the latest published version (30.20.0 as of July 23, 2025). No update is required.
marquee/src/marquee/index.js (5)
6-6: LGTM!Proper import of the WordPress block registration function.
15-15: LGTM!Correct import of SCSS styles for webpack processing, ensuring styles are applied in both editor and frontend.
20-22: LGTM!Proper imports of the block components and metadata following WordPress standards.
29-39: LGTM!Standard WordPress block registration pattern using metadata.name and properly structured edit/save components.
1-28: Excellent documentation.The comments are comprehensive and follow WordPress coding standards, providing helpful context and links to relevant documentation.
marquee/marquee.php (3)
1-16: LGTM!Plugin headers are well-structured and consistent with package.json. Version requirements are appropriate for modern WordPress block development.
18-20: LGTM!Proper security check to prevent direct file access, following WordPress best practices.
22-46: Well-implemented self-update mechanism.The conditional loading prevents conflicts between multiple plugins using the same update class, and the filter setup correctly enables auto-updates for this plugin.
marquee/src/marquee/block.json (4)
1-10: LGTM!Block metadata is properly configured with correct schema, current API version, and appropriate namespace following WordPress standards.
11-23: Well-configured block supports.The supports configuration provides good customization options while maintaining security by disabling raw HTML. The alignment restrictions to "wide" and "full" are appropriate for a marquee effect.
24-46: Attributes are well-defined.The attribute configuration is comprehensive and appropriate for a marquee block. Note that the
fadeEdgesattribute relates to the issue mentioned in the PR objectives where enabling "Fade Edges" prevents scrolling.
47-47: LGTM!Textdomain is consistent with the plugin configuration.
marquee/marquee copy.php (4)
3-16: Plugin metadata looks well-structured.The plugin header follows WordPress standards with appropriate metadata including version, requirements, author information, and licensing details. The Update URI pointing to the special projects domain is consistent with the self-update mechanism.
18-20: Security check is properly implemented.The ABSPATH check correctly prevents direct file access, which is a WordPress security best practice.
38-46: Auto-update registration follows the expected pattern.The filter hook implementation correctly adds the plugin slug to enable auto-updates from the monorepo system. The anonymous function approach is clean and self-contained.
22-28: Self-update class security review completedI’ve reviewed
marquee/classes/class-wpcomsp-blocks-self-update.phpand found no security concerns in its auto-update implementation:
- Uses WordPress HTTP API (
wp_remote_get) over HTTPS. By default SSL verification is enabled, and there’s no override disabling it.- No direct file operations (e.g.
file_put_contents,ZipArchive,WP_Filesystem, etc.) are performed in the class; it only hooks into the core update transients.- The singleton pattern and hook registration are correctly scoped to prevent duplicate loading.
No changes are required.
|
Hey @nickpagz - do you need a review here? |
|
@tommusrhodus Not yet I don't think. There's still some testing to do and a laundry list of previous issues I was made aware of/noticed after creating the PR. |
|
Hi @nickpagz , we had a request in Linear to add two enhancements to this plugin so I created a PR that does that. If you have some time to review my PR, I would really appreciate it. marquee.zip <-- updated plugin |
|
@pixel21 Apologies, finally got around to this. I approved the PR but did leave a comment wrt the fade, maybe there's an easy way to fix it. If not, we should still merge this change but track it as a separate issue. |
This PR adds the Marquee block as used on the a8c/timeline site.
marquee.zip
Summary by CodeRabbit
New Features
Documentation
Chores