-
Notifications
You must be signed in to change notification settings - Fork 111
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
Issue with missing styles when using copyStyles with @emotion #78
Comments
That's an interesting case.
…On Sun, Mar 21, 2021, 06:18 Kári Bertilsson ***@***.***> wrote:
When using emotion css lib the copyStyles={true} does not work correctly.
Emotion seems to inject css styles into DOM head at the exact time the
components are mounted. When a new popup using copyStyles={true} has
opened, it has copied all the old styles but misses the styles that were
just injected.
After closing the popup and opening it again the styles work correctly
since they are already in the DOM.
One solution might be to delay injecting the styles until the component
inside has rendered or maybe by a configurable amount of time. I guess the
delay would need to be only in the 10's ms range
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#78>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADLMH43K2PLOHYGIBXNCRTTEXBV5ANCNFSM4ZRM3UBA>
.
|
@karibertils Any chance you have an example of how to add styles to the pop-up after X ms? I tried delaying the mount of the component but it won't work. @rmariuzzo |
I'm sorry for not being more active. Currently, I'm fighting a Nodular
Sclerosis Hodgkin's Lymphoma that came back this year... and I started a
more aggressive chemo treatment.
…On gio 29 apr 2021, 03:43 Wassap124 ***@***.***> wrote:
@karibertils <https://github.com/karibertils> Any chance you have an
example of how to add styles to the pop-up after X ms? I tried delaying the
mount of the component but it won't work.
@rmariuzzo <https://github.com/rmariuzzo>
I would very much appreciate it if you'd be able to handle some of the
issues / PRs that has been piled up, the relatively low activity on this
repo is stopping it from being a lot more popular
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#78 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADLMH5W6OIJILFITYVEQQLTLEE2HANCNFSM4ZRM3UBA>
.
|
I had this need too, since I was using MUI. I was able to get it to work using the below wrapper. @rmariuzzo I'd be happy to open a PR if you think it's appropriate. Best wishes fighting the cancer. I prayed for strength and healing for you. MuiCompatNewWindow.tsx import { FC, useCallback, useRef } from "react";
import NewWindow, { INewWindowProps } from "react-new-window";
/**
* Wrapper for react-new-window that works with MUI (and Emotion).
*
* Same interface as react-new-window.
*/
const MUICompatNewWindow: FC<INewWindowProps> = ({
onOpen,
onUnload,
...props
}) => {
const mutationObserverRef = useRef<MutationObserver | null>(null);
const compatOnOpen = useCallback(
(childWindow: Window) => {
const childHead = childWindow.document.head;
const mutationObserver = new MutationObserver((mutationList) => {
mutationList
.flatMap((mutation) => Array.from(mutation.addedNodes))
.filter((node) => node instanceof HTMLStyleElement)
.forEach((styleEl) => {
childHead.appendChild(styleEl.cloneNode(true));
});
});
mutationObserver.observe(document.head, { childList: true });
mutationObserverRef.current = mutationObserver;
onOpen?.(childWindow);
},
[onOpen]
);
const compatOnUnload = useCallback(() => {
mutationObserverRef.current?.disconnect();
onUnload?.();
}, [onUnload]);
return (
<NewWindow onOpen={compatOnOpen} onUnload={compatOnUnload} {...props} />
);
};
export default MUICompatNewWindow; |
The revised code offers targeted enhancements over the original => epeterson320 comment
import { useEffect, useId } from 'react';
import NewWindow, { INewWindowProps } from 'react-new-window';
const subscriber = new Map<string, Document['head']>();
const styles = new Set<HTMLStyleElement>();
const addStyle = (style: HTMLStyleElement) => {
styles.add(style);
Array.from(subscriber.values()).forEach(head => {
head.appendChild(style.cloneNode(true));
});
};
/**
* Use this if a popup mounts and there are un-copied styles present
* onOpen={window => {
* subscriber.set(id, window.document.head);
* copyStyle(window.document.head);
* //...
* }
*
* This function is kept as a utility to handle situations where previously stored styles
* haven't been copied to the popup.
* For now, in my project, it's not being used since the existing styles are properly loaded into the popup.
*/
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const copyStyle = (target: Document['head']) => {
styles.forEach(style => target.appendChild(style.cloneNode(true)));
};
const mutationObserver = new MutationObserver(mutationList => {
mutationList.forEach(({ addedNodes, removedNodes }) => {
[...addedNodes].forEach(node => {
if (!(node instanceof HTMLStyleElement)) return;
addStyle(node);
});
[...removedNodes].forEach(node => {
if (!(node instanceof HTMLStyleElement)) return;
styles.has(node) && styles.delete(node);
});
});
});
mutationObserver.observe(document.head, { childList: true });
export default function NewWindowPopup({ onOpen, ...rest }: INewWindowProps) {
const id = useId();
useEffect(() => {
return () => {
subscriber.delete(id);
};
}, []);
return (
<NewWindow
onOpen={window => {
if (!subscriber.has(id)) subscriber.set(id, window.document.head);
onOpen?.(window);
}}
{...rest}
/>
);
} |
When using emotion css lib the copyStyles={true} does not work correctly.
Emotion injects css styles into DOM head at the time the components are mounted. When a new popup using copyStyles={true} has opened, it copies all the existing styles but misses the styles that will be injected.
This is further complicated when there is delay until a component inside the popup are mounted. For example when components need to wait for data, or won't show until state changes happen.
One solution might be to have an interval every 50-100ms monitoring when new styles get added to the parent, and then adding them to the popups
The text was updated successfully, but these errors were encountered: