Skip to content
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

Add a Marquee block #13

Draft
wants to merge 17 commits into
base: trunk
Choose a base branch
from
Draft

Add a Marquee block #13

wants to merge 17 commits into from

Conversation

michalczaplinski
Copy link
Collaborator

@michalczaplinski michalczaplinski commented Jul 15, 2024

This adds a Marquee block that is capable of scrolling the contents that it contains. The block takes a Paragraph, Heading, Button, or Image block as children.

🚧 It's a WIP

Copy link
Collaborator Author

@michalczaplinski michalczaplinski left a comment

Choose a reason for hiding this comment

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

I took a look at the approach of using a Block Variation of the Group block to implement this.

Unfortunately, the Block Variations API seems insufficient to pull this off. We'd need a few things that it does not provide AFAIK:

  • A way to customize the CSS used by the block (for the scrolling animation)
  • A way to wrap the block with another HTML element that serves as a scroll container

@michalczaplinski
Copy link
Collaborator Author

michalczaplinski commented Jul 17, 2024

I got a bit stuck on how to "clone" the inner blocks of the Marquee correctly.

For a marquee animation to work, the inner contents of the Marquee block need to be duplicated so that the animation gives an illusion that the same element is scrolling infinitely. Here's an example from Nectar blocks:

output_bac22c.mp4

At the moment, multiple Inner blocks are not possible (WordPress/gutenberg#6808), so I think I'll have to resort to cloning the element on the front end.

@michalczaplinski
Copy link
Collaborator Author

I could also duplicate the inner content using the HTML API. But this means converting the block to a dynamic one. It feels a bit like overkill but it's probably the right way to go.

@michalczaplinski
Copy link
Collaborator Author

Oh, I think I don't actually need the HTML API, I can render the $content twice in a dynamic block. Simplified version of what I need:

<?php
$inner_blocks = $block->inner_blocks;
$inner_blocks_html = '';
foreach ($inner_blocks as $inner_block) {
    $inner_blocks_html .= $inner_block->render();
}

?>

<div <?php echo get_block_wrapper_attributes(); ?> >
	<div className="wp-block-wpcomsp-marquee__inner"><?php echo $inner_blocks_html; ?></div>
	<div className="wp-block-wpcomsp-marquee__inner"><?php echo $inner_blocks_html; ?></div>
</div>

@Nyiriland
Copy link

I got a bit stuck on how to "clone" the inner blocks of the Marquee correctly.

From experience creating marquees, I've always used js to measure the parent, then measure the content, then duplicate content as necessary to fill the parent. Is it possible to measure those widths without js after the CSS has been rendered?

@michalczaplinski
Copy link
Collaborator Author

michalczaplinski commented Jul 17, 2024

Oh, I think I don't actually need the HTML API, I can render the $content twice in a dynamic block. Simplified version of what I need:

I was wrong here again - I would need to get the block props from the get_block_wrapper_attributes() twice and again this is not possible.

Is it possible to measure those widths without js after the CSS has been rendered?

It's not possible 🙂

I'm back to using JS on the frontend to clone the inner elements of the Marquee block:

// view.js 

store("wpcomsp/marquee", {
	callbacks: {
		init: () => {
			const { ref } = getElement();
			const clonedElement = ref.firstElementChild.cloneNode(true);
			ref.appendChild(clonedElement);
		},
	},
});

From experience creating marquees, I've always used js to measure the parent, then measure the content, then duplicate content as necessary to fill the parent

Thanks for the tip! Is it necessary to measure the parent, though? I'm cloning the inner blocks once and animating the whole parent container. So, if you (for example) want more space between the items, you modify the parent's layout. It looks something like this:

output_f1833f.mp4

@Nyiriland
Copy link

Nyiriland commented Jul 17, 2024

Thanks for the tip! Is it necessary to measure the parent, though? I'm cloning the inner blocks once and animating the whole parent container.

I think you do, because you don't want all that white space, you want to hug the child items together. I think you can animate the parent container as long as it hugs the width of its child items; then you can easily duplicate the parent. Also keep in mind block spacing: you'll want to apply that setting evenly between all blocks.

I added this in the Figma file if needed. :)

image

@michalczaplinski
Copy link
Collaborator Author

Ah, yeah, that makes sense, thank you! 🙂

@michalczaplinski
Copy link
Collaborator Author

I made some more progress:

output_8592d6_2.mp4

I might have to add a placeholder because en empty Marquee (without any inner blocks) looks just like an empty space (see this comment).

I'm considering whether the Marquee block should contain a single inner Marquee contents block. This way, we could optimize the animation performance. Currently, the animation is attached to every child block of the Marquee and they are all animated in unison. If there are many children, this can cause some perf issues, dropped frames, etc.

@Nyiriland
Copy link

Nyiriland commented Jul 18, 2024

I might have to add a placeholder because en empty Marquee (without any inner blocks) looks just like an empty space

Agreed. Maybe we can just add an h2 with the text "Add Marquee content here..." as placeholder.

I know i was just thinking about text, buttons, and images, but I really like that we can any block into this version of the Marquee! If it's really just a scrolling row block, that makes sense.

This is what the architecture should look like, IMO. The "content wrapper" is the content of the block, which will get repeated:
image

For gaps, the marquee's "block spacing" setting should determine the spacing between its children. It will need to be applied to both the marquee content, and the gap between the repeated content.

@michalczaplinski
Copy link
Collaborator Author

I have to wrap up for today, but here's the status update:

I figured that I'll need the Marquee to have a single Marquee Contents inner block (like mentioned at the end of my previous comment). This way the Marquee block is separate from the "content wrapper" - the latter is the one we want to animate.

PS. @Nyiriland That's the exact same idea as what you've described. The Marquee block is what you decribed as the "Marquee block container" and "Content wrapper" is the Marquee Contents.

The way I want to accomplish this is with a Block Variation, but I ran into 2 issues:

  1. I keep getting the Your site doesn’t include support for the "wpcomsp/marquee-content" block message, even though the block shows up in the inserter and can be inserted. Example video:

    output_076d44.mp4
  2. When I do insert the Marquee Contents block into the the editor, the Row variation of the Group block seems to take priority over my variation and the editor recognizes it as such. I know that the Marquee Contents should be restricted to only be accepted inside of the Marquee block, but this is just for demonstration:

    output_61b481.mp4

@jffng
Copy link

jffng commented Jul 18, 2024

I keep getting the Your site doesn’t include support for the "wpcomsp/marquee-content" block message, even though the block shows up in the inserter and can be inserted. Example video:

I may have ran into this issue also, it was related to WordPress/gutenberg#62202.

I worked around it by pinning @wordpress/scripts to ^27.9.0 in my dev dependencies. I don't think that's ideal, but I had already lost so much time to the blocks not showing up.

@jarekmorawski
Copy link

Not sure if that helps at this stage, but it'd be amazing to let users see the animation in the site editor without opening the preview in a new tab. What if we triggered the interaction when the block is not selected? Selecting it with make the content visible and editable; deselecting would trigger the preview mode again.

@gziolo
Copy link
Collaborator

gziolo commented Jul 19, 2024

I may have ran into this issue also, it was related to WordPress/gutenberg#62202.

I worked around it by pinning @wordpress/scripts to ^27.9.0 in my dev dependencies. I don't think that's ideal, but I had already lost so much time to the blocks not showing up.

This is what npx @wordpress/create-block does by defualt now, pins the @wordpress/scripts version to the older version that works out of the box with every latest major WP version. That's why in #12, I have ^27.9.0 specified which works with no issues. The latest version of wp-scripts works only with WP 6.7 because of the changes introduced in how JSX gets transpiled by the build. Unfortunately, this situation is very confusing for many developers, but it will require a few more months before that default configuration becomes seamless again.

I also see that I will have to enable the experimental support for ES Modules for the view script to use Interactivity API with --experimental-modules. That's another friction point that complicates the development efforts.

@Nyiriland
Copy link

Not sure if that helps at this stage, but it'd be amazing to let users see the animation in the site editor without opening the preview in a new tab. What if we triggered the interaction when the block is not selected?

I agree a preview of the scrolling would definitely be ideal to show in the editor, but to comply with a11y best practices, the motion should be user-triggered. I think we could include a toggle in the toolbar and sidebar; some explorations below.

@jarekmorawski please jump in if you know of design precedence for this kind of setting!

image image

@jarekmorawski
Copy link

Oh, I like that! I can't think of an existing precedence. The closest that comes to mind is view-related settings in the editor's preferences. 🤔 The toolbar could work, though I wonder if it'd be an icon button instead.

image

Alternatively, we'd place the toggle in the Advanced section, just like the Force page reload option in the Query Loop block. That, too, isn't fully reflected in the editor's canvas.

image

We'd also try something similar to what I explored a while ago for content previews for page templates. It'd likely be a larger lift because no such UI exists in the editor yet.

image

Ideally, we'd have a global setting that would apply to the entire experience, from blocks to the editor's UI.

image

@michalczaplinski
Copy link
Collaborator Author

I agree a preview of the scrolling would definitely be ideal to show in the editor

That sounds great, I'll try to incorporate this today. I think that the Markdown block in P2 and the Custom HTML blocks both have a "preview" mode so we can use a similar UI (in the block's toolbar)

@michalczaplinski
Copy link
Collaborator Author

The issue with Your site doesn’t include support for the "wpcomsp/marquee-content block" does not seem to go away after pinning the wp-scripts dependency. I suspect that the problem here is different - I will ask for more help 🙂

@michalczaplinski
Copy link
Collaborator Author

I figured out the issue with the Your site doesn’t include support for the "wpcomsp/marquee-content" block. I needed to pass the name of the actual block to allowedBlocks and not the name of the variation:

// This works
const innerBlocksProps = useInnerBlocksProps(blockProps, {
	allowedBlocks: ["core/group"],
	...
});

// This DOES NOT work
const innerBlocksProps = useInnerBlocksProps(blockProps, {
	allowedBlocks: ["wpcomsp/marquee-content"],
	...
});	

The issue I’m running into now is twofold:

  • The editor does not unregister the Row and Group blocks. I’m not sure what I’m doing wrong here?
  • My wpcomsp/marquee-content block is still not inserted correctly. I can select it from the inserter, but it transforms into the Row block upon inserting it!

@michalczaplinski
Copy link
Collaborator Author

I could not find a better solution than just using setTimeout() which gets the job done but is almost certainly not the long-term solution here:


setTimeout(() => {
	unregisterBlockVariation("core/group", ["group-row"]);

	registerBlockVariation("core/group", {
		name: "wpcomsp/marquee-content",
		title: "Marquee Content",
		icon: "editor-table",
		description: __("A marquee content block."),
		scope: ["block"],
		attributes: {
			layout: { type: "flex", flexWrap: "nowrap" },
			className: "wpcomsp-marquee-content",
			allowedBlocks: [
				"core/paragraph",
				"core/heading",
				"core/image",
				"core/buttons",
			],
		},
		isActive: (blockAttributes) =>
			blockAttributes.layout?.type === "flex" &&
			(!blockAttributes.layout?.orientation ||
				blockAttributes.layout?.orientation === "horizontal"),
	});

	// re-register the Row block variation
	registerBlockVariation("core/group", {
		name: "group-row",
		title: _x("Row", "single horizontal line"),
		description: __("Arrange blocks horizontally."),
		attributes: { layout: { type: "flex", flexWrap: "nowrap" } },
		scope: ["block", "inserter", "transform"],
		isActive: (blockAttributes) =>
			blockAttributes.layout?.type === "flex" &&
			(!blockAttributes.layout?.orientation ||
				blockAttributes.layout?.orientation === "horizontal"),
		icon: row,
	});
}, 1000);

Another thing that I'm uncertain about is the re-registering the Core variations. At the moment, I just copy-pasted the definition from Gutenberg but that definition can change at any moment 🙂.

@michalczaplinski
Copy link
Collaborator Author

I need to wrap up work on the block from my side.

Unfortunately, I didn't get far enough to implement the preview. However, it shouldn't be difficult to extend the block from its current state.

I'm happy that I could re-use the mechanics of the Row block for the inner contents of the Marquee. This way, it's possible to use all the same global styles to style the contents of the Marquee as in the Row block. That said, the implementation is hacky and should be reviewed to avoid the hacks I documented above. The most immediate next steps for whoever takes over this work would be:

  1. Figure how to properly de-register and register the block variations as described in Add a Marquee block #13 (comment)
  2. Make sure that only a select subset of blocks can be added as children of the Marquee Content block
  3. Add a preview mode as discussed in Add a Marquee block #13 (comment)
  4. Figure out if we can prevent the users from setting the Allow to wrap to multiple lines on the Marquee Content block. This might have to be done in Gutenberg, as I see that there is no global setting that controls it: https://github.com/WordPress/gutenberg/blob/34e18e62dd9637f1ffb40cd64f91e6ee55704fce/packages/block-editor/src/layouts/flex.js#L348-L363

@Nyiriland
Copy link

Some other notes:

  • Block Icon – @Nyiriland to do
  • Rename to "Scrolling Marquee" – I feel including "scrolling" in the name would be a little more obvious for people who don't know the history of the html marquee element

@philcable
Copy link

On the off chance that this even a little bit helpful, we implemented a marquee as a Group block style for a recent project. We didn't do anything for the editor view, but here's what we did for the front-end:

add_filter( 'render_block_core/group', 'wpcomsp_marquee_group_block', 10, 2 );

/**
 * Filter output for marquee style group blocks.
 *
 * @param string $block_content Block content.
 * @param array  $block         The full block.
 *
 * @return string Modified block content.
 */
function wpcomsp_marquee_group_block( string $block_content, array $block ): string {
	if ( 'is-style-marquee' === ( $block['attrs']['className'] ?? false ) ) {
		$tag_name  = $block['attrs']['tagName'] ?? 'div';
		$classlist = '';
		$styles    = '';

		$tags = new \WP_HTML_Tag_Processor( $block_content );

		// Capture the class and style attribute values before removing them.
		if ( $tags->next_tag( $tag_name ) ) {
			$classlist = $tags->get_attribute( 'class' );
			$styles    = $tags->get_attribute( 'style' );

			$tags->remove_attribute( 'class' );
			$tags->remove_attribute( 'style' );
		}

		// Capture the original block without wrapper attributes to use as content.
		$content = $tags->get_updated_html();

		$block_content = sprintf(
			'<%1$s class="%2$s" style="%3$s">%4$s%4$s</%1$s>',
			$tag_name,
			$classlist,
			$styles,
			$content
		);
	}

	return $block_content;
}

And the styles:

/* --- Marquee style - Group block --- */
.is-style-marquee {
	--marquee-gap: var(--wp--preset--spacing--13);
	display: flex;
	gap: var( --marquee-gap );
	overflow: hidden;
	position: relative;
	user-select: none;

	> * {
		animation: marquee 35s linear infinite;
		flex-shrink: 0;
		display: flex;
		gap: var(--marquee-gap);
		justify-content: space-around;
		margin: 0;
		min-width: 100%;
	}

	&:hover {
		> * {
			animation-play-state: paused;
		}
	}
}

@media (prefers-reduced-motion: reduce) {
	.is-style-marquee {
		> * {
			animation-play-state: paused !important;
		}
	}
}

@keyframes marquee {
	from {
		transform: translateX(0);
	}
	to {
		transform: translateX(calc(-100% - var(--marquee-gap)));
	}
}

@sagarnasit
Copy link

Hi @Nyiriland @michalczaplinski

I require a marquee block on https://github.com/a8cteam51/blanes-museum project. I found out we already have a draft pull request for the block. Can anyone share the status of the block development? Let me know if you need help taking this block development further.

@Nyiriland
Copy link

Hey @sagarnasit! This work on this block was done as part of a 2-week sprint. I would love to move it forward but it's difficult to find resources, but we should get @stronenv's opinion on that.

@stronenv
Copy link

Hey @sagarnasit ! Thanks for flagging. If you need a marquee for the Blanes Museum feel free to extend on this block, that's great!

cc: @nate-allen

@tommusrhodus
Copy link
Contributor

@Nyiriland @stronenv Is the current status of this block then that it's looking for a dev?

@Nyiriland
Copy link

Nyiriland commented Oct 30, 2024

@tommusrhodus I believe so, but let's tag in our project manager @dhanson-wp to confirm.

@dhanson-wp
Copy link

@tommusrhodus that is correct. This block needs a dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Build 🏗
Development

Successfully merging this pull request may close these issues.

10 participants