Open Swanand01 opened 1 month ago
I've tried to replicate the following functionality using the Interactivity API:
Using the Interactivity API requires adding a viewScript
in the block.json
file, so I have placed the view.js
file in the same folder as the block.json
file.
This file mirrors the web-stories-lightbox.js
file, which was used to perform the above-mentioned functionality. The main difference here, is that we call actions
directly, instead of using event listeners.
These actions
are defined in view.js
, and are called from the DOM elements in Renderer.php
.
I think we'll need to add these attributes to the <amp-story-player />
in Embed.php
and Singleton.php
as well.
WDYT @swissspidy ?
It took me a moment to realize that the interactivity API somehow doesn't work with classic themes... I'm probably missing something, but I opened https://core.trac.wordpress.org/ticket/61387 just in case. But if it's truly a bug, then this is a blocker for us.
Implementation-wise it looks like a nice port of the existing script 👍 It's also super snappy.
Functionality-wise, later on we could try to convert the carousel script to this new format as well. I just saw a similar solution for a slider here: https://github.com/ryanwelcher/iapi-gallery-slider/blob/38b63eacf018f29153980fce7d7278d7b41f1011/src/view.js
So a carousel like ours could potentially also be built. But that's future talk :)
What's your experience so far with using this interactivity API?
Note: For the file location we'll have to come up with something different eventually as I don't want to put JS files into the blocks
folder. Maybe this could be a new stories-block-view
package under packages/
and then we copy some files with webpack if necessary. Then we could try to use TypeScript as well.
I think we'll need to add these attributes to the
<amp-story-player />
inEmbed.php
andSingleton.php
as well.
For Singleton
, yes, as it is used to display a single story's poster image and then when you click on it, the player opens in a lightbox.
For Embed
, no, as that one directly displays the player for a given story and does not use a lightbox at all.
In the editor you can switch between the two modes using this toggle:
Another thing we need to be careful: we don't need to add this to the AMP version of the markup (everything wrapped in $this->context->is_amp()
), as AMP handles the interactivity for us there (i.e. if one is using the AMP plugin)
It took me a moment to realize that the interactivity API somehow doesn't work with classic themes... I'm probably missing something, but I opened core.trac.wordpress.org/ticket/61387 just in case. But if it's truly a bug, then this is a blocker for us.
OK it's my fault for not reading the docs right.
So we'll definitely need to move this script to packages/stories-block-view
and then add a corresponding entry to the webpack.config.cjs
. This way, there will be an .asset.php
file generated for the script.
For classic themes in particular we'll also need wp_interactivity_process_directives
for server-side processing of these directives
What's your experience so far with using this interactivity API?
I think it is a nice addition to have, and makes things simpler as I don't have to add event listeners manually. I also like the declarative approach. However, I also feel like it's not very intuitive, one can't really figure out what's going on by just looking at element attribute.
Hi @swissspidy, I have created a new package stories-block-view
, and added an entry in the webpack.config.js
file as well.
For classic themes in particular we'll also need wp_interactivity_process_directives for server-side processing of these directives
How and where should this be implemented in our Renderer
? Do I check if the theme is a classic theme, and then pass the stringified markup through wp_interactivity_process_directives
, and then render that instead?
How and where should this be implemented in our
Renderer
? Do I check if the theme is a classic theme, and then pass the stringified markup throughwp_interactivity_process_directives
, and then render that instead?
I think it would need to be added at the end of the Carousel_Renderer::render()
and Generic_Renderer::render()
methods, before the output is running through a filter. Probably don't even need to check the theme, just call the function if it exists.
Size Change: +8.97 kB (+0.33%)
Total Size: 2.77 MB
Filename | Size | Change | |
---|---|---|---|
assets/js/web-stories-block-view.js |
8.91 kB | +8.91 kB (new file) | 🆕 |
Hi @swissspidy, the Build & deploy action seems to be failing:
ERROR in ./packages/stories-block-view/src/index.js 1:40-136
Module not found: Error: Attempted to use WordPress script in a module: /home/runner/work/web-stories-wp/web-stories-wp/node_modules/react-refresh/runtime.js, which is not supported yet.
ERROR in ./node_modules/@pmmmwh/react-refresh-webpack-plugin/lib/runtime/RefreshUtils.js 2:14-46
Module not found: Error: Attempted to use WordPress script in a module: react-refresh/runtime, which is not supported yet.
@ ./packages/stories-block-view/src/index.js 117:37-60
ERROR in ./node_modules/@pmmmwh/react-refresh-webpack-plugin/client/ReactRefreshEntry.js 4:23-55
Module not found: Error: Attempted to use WordPress script in a module: react-refresh/runtime, which is not supported yet.
Do I need to change something in the webpack.config.cjs
file?
Particularly in :
const webStoriesBlockView = {
...sharedConfig,
output: { ...sharedConfig.output, module: true },
experiments: { outputModule: true },
entry: {
'web-stories-block-view': './packages/stories-block-view/src/index.js',
},
plugins: [
...sharedConfig.plugins.filter(
(plugin) => !(plugin instanceof DependencyExtractionWebpackPlugin)
),
// React Fast Refresh.
!isProduction && new ReactRefreshWebpackPlugin(),
new WebpackBar({
name: 'Web Stories Block View',
color: '#a9db14',
}),
new DependencyExtractionWebpackPlugin(),
].filter(Boolean),
};
This might be of help.
Yeah need to remove the ReactRefreshWebpackPlugin
line as this is not a React script.
Plugin builds for 8f9577b680c883656891ea4afce28258d4c7fe69 are ready :bellhop_bell:!
Hi @swissspidy, this test is failing for PHP > 8.3:
Google\Web_Stories\Tests\Integration\Media\Types::test_get_allowed_mime_types_multisite
--- Expected
+++ Actual
@@ @@
2 => Array (...)
3 => Array (
0 => 'audio/mpeg'
- 1 => 'audio/wav'
- 2 => 'audio/ogg'
+ 1 => 'audio/aac'
+ 2 => 'audio/wav'
+ 3 => 'audio/ogg'
)
4 => Array (...)
)
Interestingly, this test passed when run locally. My PHP version is 8.3.0, with WP 6.5.4
It‘s not the PHP version. It‘s because it‘s testing against WordPress trunk. It will probably fix itself in trunk. If not, we can open an issue for it, as it is unrelated to this PR.
Revisiting this:
The Interactivity API approach looks appealing, but given the duplicate code required to still support WP < 6.5, let's put this aside until we raise our minimum version requirement. Then we should be able to remove some of that code, and rewrite it in TypeScript too.
Summary
This PR makes use of the new Interactivity API to implement the lightbox functionality in the Web Stories block.
On WP versions < 6.5
packages/stories-lightbox/src/web-stories-lightbox.js
would continue being used to handle the functionality of opening and closing stories.On WP versions > 6.5
packages/stories-block-view
contains the necessarystate
andactions
To learn more about these, please visit General overview of the Interactivity API
User-facing changes
None. The user won't notice a difference in how the Stories are rendered and opened.
Testing Instructions
This PR can be tested by following these steps:
data-wp-interactive
Reviews
Does this PR have a security-related impact?
No.
Does this PR change what data or activity we track or use?
No.
Does this PR have a legal-related impact?
No.
Checklist
Type: XYZ
label to the PRFixes #13502