-
Notifications
You must be signed in to change notification settings - Fork 22
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
Sidebar and modal styling fixes #8234
Conversation
@@ -5,7 +5,10 @@ | |||
all: initial; | |||
|
|||
// Set a font baseline style. Bootstrap targets `body` specifically, which isn't available in a shadow DOM | |||
font: 16px / 1.5 sans-serif; | |||
font: |
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.
fixes the font issue. Tried to use a variable form _variables.scss
, but ran into errors. Probably not worth worrying about since it's unlikely to change.
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.
Could you reference the variable from _variables.scss
so we know what it's copied from? Or does the variable not exist yet?
@@ -68,6 +68,7 @@ const IframeModal: React.VFC<{ | |||
src={url.href} | |||
title="Modal content" | |||
style={{ | |||
all: "initial", |
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.
Somehow left out of the previous PR. Fixes the opacity issue
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.
This is already part of renderWidget's element. What's the issue here? I don’t think I removed/changed this in that PR
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.
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.
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 the issue is that my renderWidget
PR removed the shadow DOM because it's supposed to only be part of the IsolatedComponent
. This week I'll send a PR combining these two new pieces.
@@ -123,7 +123,7 @@ const store = configureStore({ | |||
/* eslint-disable unicorn/prefer-spread -- It's not Array#concat, can't use spread */ | |||
return getDefaultMiddleware({ | |||
...defaultMiddlewareConfig, | |||
immutableCheck: false, | |||
serializableCheck: false, |
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.
Makes the Extension Console significantly more performant in dev mode. No impact on production.
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.
immutable check is already built into the default middleware config
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.
immutable check is already built into the default middleware config
👍 You mean it's already off in the default middleware config?:
const defaultMiddlewareConfig = { |
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.
Yes, that's exactly what I meant
No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack. Do not edit this comment manually. |
What does this PR do?
<IsolatedComponent>
forDocumentView
,EphemeralFormContent
,CustomFormComponent
#8200Demo
Checklist