refactor: refactor overlay styling to use css class #4861
Conversation
malcolm-kee
requested review from
alexander-akait,
hiroppy,
snitin315 and
anshumanv
as code owners
| * @param {Document} doc | ||
| * @return {CssLoader} | ||
| */ | ||
| const createCssLoader = (doc) => { |
There was a problem hiding this comment.
css loader could be confusing? Any suggestion is welcome.
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #4861 +/- ##
==========================================
+ Coverage 90.38% 90.50% +0.11%
==========================================
Files 16 16
Lines 1706 1706
Branches 649 649
==========================================
+ Hits 1542 1544 +2
+ Misses 150 148 -2
Partials 14 14 ☔ View full report in Codecov by Sentry. |
| @@ -129,12 +142,10 @@ const createOverlay = (options) => { | |||
| applyStyle(iframeContainerElement, iframeStyle); | |||
There was a problem hiding this comment.
Intentionally keep iframe styling to use inline style because we can't scope CSS for it.
| css: /* css */ `.webpack-msg-text { | ||
| line-height: 1.5; | ||
| font-size: 1rem; | ||
| font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace; |
There was a problem hiding this comment.
Lovingly copied from TailwindCSS font-mono: https://tailwindcss.com/docs/font-family
| loadedCss.push(css); | ||
| } | ||
| }, | ||
| }; |
There was a problem hiding this comment.
It can create a lot of problems. Shorty - we already tried this solution and we had a lot of issues about style tags and its order and overlapping and etc and a lot, so I prefer do not use style and classes at all, I am fine with refactor and move styles in own objects with good names, but using classes again make more problems than benefits
There was a problem hiding this comment.
Got it. Any issues that was raised before on this? Maybe worth include them in the comments
There was a problem hiding this comment.
They were, but they are difficult to find because they are scattered and many headings do not match the original problem, I tried to search for them, but unfortunately I can not find but I remember very well that we had problems with this 😄
There was a problem hiding this comment.
No worries, tag me when you stumble them then.
For Bugs and Features; did you add new tests?
N/A
Motivation / Use-Case
This PR refactor styling for overlay from inline styles to CSS class, which provides the following benefits:
Breaking Changes
N/A
Additional Info
This is prepare for better overlay styling that I show in #3689 (comment)