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

Stabilize BlockCanvas's contentRef property #67209

Open
costasovo opened this issue Nov 21, 2024 · 1 comment · May be fixed by #67210
Open

Stabilize BlockCanvas's contentRef property #67209

costasovo opened this issue Nov 21, 2024 · 1 comment · May be fixed by #67210
Assignees
Labels
[Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@costasovo
Copy link
Contributor

costasovo commented Nov 21, 2024

What problem does this address?

The contentRef property, which provides access to the editor canvas wrapping element, is currently available only in the ExperimentalBlockCanvas. This prevents third parties which use the BlockCanvas to access the canvas element via the ref.

The property is used in the VisualEditor component in wordpress/editor package to handle various visual effects, focusing blocks or handling clicks.

There is a similar use case in the Email Editor we are building for MailPoet. The editor is currently experimental, but we plan to release in the next couple of months so we want to eliminate usage of private APIs as much as possible.

Another example of usage is in IsolatedBlockEditor.

It seems that a couple of independent custom editors ended up using this property.

What is your proposed solution?

I propose exposing the contentRef property on the public BlockCanvas component PR:(#67210).

Stabilizing the contentRef prop was already proposed as part of #57672. I want to highlight the comment from @youknowriad.

BlockCanvas shouldIframe, iframeProps contentRef
I intentionally kept these props private. They were needed because of our weird implementations and backward compatibility needs in WordPress but from a third-party developer perspective, I would love for the iframe to be default (no need to provide a choice), and for the other props to be not needed. I didn't want to commit to them as API because they felt very unclear from an implementer's perspective.

It has been more than half a year since the proposal so I hope we could revisit this. Using ref/forwardRef is quite common so I wouldn't say it is unclear. My concern here that allowing access to a dom element as public API could be too powerful and hard to manage in terms of backward compatibility. What would be the alternative approach?

Pros

  1. ref forwarding is known React pattern
  2. it is easy to do

Cons

  1. exposing a dom node to third parties allows variety of modifications and we can't fully garant backward compatibility as any change in the dom can potentially break something. We could perhaps address this by limiting the ref via useImperativeHandle
@costasovo costasovo added the [Type] Enhancement A suggestion for improvement. label Nov 21, 2024
@costasovo costasovo linked a pull request Nov 21, 2024 that will close this issue
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Nov 21, 2024
@youknowriad
Copy link
Contributor

Cn you share what use-cases you're trying to achieve by exposing the content ref? Trying to understand the problem rather than the solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants