-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Layout: use override editor style API #54466
Conversation
Size Change: +23 B (0%) Total Size: 1.53 MB
ℹ️ View Unchanged
|
return () => { | ||
deleteStyleOverride( id ); | ||
}; | ||
}, [ id, css, setStyleOverride, deleteStyleOverride ] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it makes sense to have a useStyleOverride
hook as this seems to be common right? At least an internal one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe!
element && | ||
!! css && | ||
createPortal( | ||
<LayoutStyle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what replaced this component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh looking at the component itself, it does nothing if it has a "css" prop, it just renders that css. Makes me think we should remove that css prop from the LayoutStyle component and just use <style>
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
What?
Uses the API introduced in #52888 for Layout.
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast