Skip to content

🏗 [bento] Add portals to preact compat#33213

Merged
krdwan merged 2 commits intoampproject:masterfrom
krdwan:sidebar
Mar 17, 2021
Merged

🏗 [bento] Add portals to preact compat#33213
krdwan merged 2 commits intoampproject:masterfrom
krdwan:sidebar

Conversation

@krdwan
Copy link
Copy Markdown
Contributor

@krdwan krdwan commented Mar 11, 2021

Add portals to preact/compat for use in Bento components

@krdwan krdwan requested review from caroqliu and jridgewell March 11, 2021 20:03
@krdwan krdwan marked this pull request as ready for review March 11, 2021 21:00

/**
* @param {PreactDef.VNode} vnode
* @param {Element} container
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see the Preact.createPortal takes a PreactElement type for container which extends HTMLElement. I don't think our PreactDef has anything like PreactElement, @jridgewell should we add this to our externs, use HTMLElement, or is this ok as is?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

HTMLElement is fine. They use PreactElement so they can attach private props to the element, but we don't need to know about that.

@krdwan krdwan requested a review from alanorozco March 16, 2021 13:59

/**
* @param {PreactDef.VNode} vnode
* @param {Element} container
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

HTMLElement is fine. They use PreactElement so they can attach private props to the element, but we don't need to know about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants