Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Support providing Block Context from non-interactive blocks #28

Merged
merged 34 commits into from
Jul 26, 2022

Conversation

michalczaplinski
Copy link
Collaborator

⚠️ This is still WIP

We have added support for BlockContext in #7.

Interactive blocks can now provide block context to other interactive blocks on the frontend.

However, static (not hydrated) blocks cannot provide context to interactive blocks. This PR is adding support for this feature.

@michalczaplinski michalczaplinski force-pushed the feature/static-context branch from f839d1e to 3a3b9f3 Compare May 24, 2022 17:24
@michalczaplinski
Copy link
Collaborator Author

I've recorded a brief overview of what we've been trying to achieve. If anybody would like to continue this work, you're very welcome to do so!

2022-05-24_16-59-49.mp4

@luisherranz
Copy link
Member

Context is not available on save, but why do you need it? If a block provides a context, the value is in one of its attributes. We just need to serialize that value, then add a global event listener.

@michalczaplinski
Copy link
Collaborator Author

Our idea was simply:

  • Read the context that is passed from the static parent in the save() of the interactive block.
  • Serialize it in and add it to the attribute like in the save().
  • Deserialize and use in the frontend.

But you're right that with the context not being available on save() it's not going to work.

If a block provides a context, the value is in one of its attributes. We just need to serialize that value, then add a global event listener.

Yup, but I think that would require some modification to the "static" blocks, like adding a wrapper around the save() of the static blocks and we were trying to avoid that. Unless you had a different idea here? 🤔

@luisherranz
Copy link
Member

like adding a wrapper around the save() of the static blocks

I don't really care if it's a wrapper or a blocks.getSaveContent.extraProps filter, as long as it is abstracted by Gutenberg and not exposed to the developer 🙂

@michalczaplinski michalczaplinski force-pushed the feature/static-context branch from 3046a44 to 48c45fa Compare June 29, 2022 16:06
@michalczaplinski
Copy link
Collaborator Author

We paired up to pair-program on this PR with @luisherranz.

We still have to figure out where and how to serialize the data in the static (non-interactive) block:

image

@luisherranz luisherranz changed the title WIP: Add support for "static" context Support providing context from non-interactive blocks Jun 30, 2022
@michalczaplinski
Copy link
Collaborator Author

I was able to make it work so far in the simplest case: one static (non-interactive) parent and one child interactive block 👍 .

Most immediate next steps would be:

  • Making it work with multiple parents. An interactive block can inherit context from multiple parent blocks.
  • Making it work with initially hidden blocks. Adding a global event listener on load does not work because if a new interactive block is dynamically added to the DOM, it will not be subscribed to the events! I guess we have to encapsulate this code in the connectedCallback() of our <gutenberg-interactive-block> custom component.

@michalczaplinski
Copy link
Collaborator Author

I made some more progress:

  • Got it to work with multiple parents
  • Got it to work with initially hidden blocks

(basically, what I have mentioned in the comment above)

I'm sure it's still buggy but seems to work at least in the happy path.

2022-07-06_19-24-35.mp4

@luisherranz
Copy link
Member

I'm resolving the conflicts with my refactor on d61391c.

@luisherranz
Copy link
Member

@michalczaplinski everything should be working again now. Feel free to continue with this 🙂

@luisherranz luisherranz self-assigned this Jul 25, 2022
@luisherranz luisherranz dismissed their stale review July 25, 2022 14:20

The attributeFilter approach didn't work. We're going with Michal's initial custom-element proposal.

@luisherranz
Copy link
Member

As the Mutation Observer option could introduce performance concerns and the attributeFilter approach didn't work, I just renamed gutenberg-interactive-block to gutenberg-block and reused all the code.

I've also renamed all the blocks. I hope the new names will help to make more sense of the structure of these experiments for new people looking into them.

I've made a video to explain the changes.

https://www.loom.com/share/72d45ab8574c42258e8eee48c3dca3d5

@michalczaplinski would you mind taking a look? Thanks!! 🙂

@luisherranz
Copy link
Member

I'm going to resolve the conflicts with #41.

@luisherranz
Copy link
Member

Done 👍️

AFAIK, everything should be ready to merge now.

@luisherranz luisherranz force-pushed the feature/static-context branch from 63554d4 to 48f88aa Compare July 25, 2022 15:54
@michalczaplinski
Copy link
Collaborator Author

Looks great, thanks for working on this, @luisherranz ! 🙂

I can't approve my own PR just going to merge it.

@michalczaplinski michalczaplinski merged commit c74f76e into main Jul 26, 2022
@luisherranz luisherranz deleted the feature/static-context branch July 26, 2022 07:11
@luisherranz
Copy link
Member

Thanks, Michal! 🎉

@luisherranz luisherranz changed the title Support providing context from non-interactive blocks Support providing Block Context from non-interactive blocks Jul 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants