Skip to content
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

[BUG] ReactTooltip isn't being rendered in Shadow DOM mode #1029

Open
dsternlicht opened this issue May 23, 2023 · 25 comments
Open

[BUG] ReactTooltip isn't being rendered in Shadow DOM mode #1029

dsternlicht opened this issue May 23, 2023 · 25 comments
Labels
Bug Help Wanted V6 It might get fixed/merged before, but most likely only on V6's release.

Comments

@dsternlicht
Copy link

dsternlicht commented May 23, 2023

Describe the bug
My React app is being rendered in a Shadow DOM, ReactTooltip isn't working. If I'm disabling the Shadow DOM mode, it's working as expected.

Version of Package
v5.13.1

To Reproduce

  • App.tsx
import { Tooltip as ReactTooltip } from 'react-tooltip';
import { StyledDiv } from './styles';

export const App = () => {
	return (
		<StyledDiv>
			<ReactTooltip
				id={`my-tooltip`}
				style={{ zIndex: 100, maxWidth: 300, fontSize: 13 }}
			/>
		</StyledDiv>
	);
};
  • index.tsx
const shadowRoot = document.body.attachShadow({ mode: 'open' });
	
// create a root inside the shadow
const root = ReactDOM.createRoot(shadowRoot);

// Render the new widget
root.render(<App />);

Expected behavior
Placeholder element should appear in the dom. Tooltips should work.

Desktop (please complete the following information if possible or delete this section):

  • OS: MacOS
  • Browser Chrome
  • Version latest
  • Frameworks React 18.2.0, Next.js 13.1.1
@gabrieljablonski
Copy link
Member

gabrieljablonski commented May 23, 2023

Can you please try react-tooltip@5.12.0?

v5.13.0 introduced automatic CSS injection for the default tooltip styling, so that might've broken this use case.

For this test you'll have to add the CSS import with:

import 'react-tooltip/dist/react-tooltip.css'

@dsternlicht
Copy link
Author

@gabrieljablonski I've just tried that, still nothing. The placeholder doesn't even appears in the DOM.

@danielbarion
Copy link
Member

@dsternlicht can you please provide a reproducible example of the issue? (can be a repository)

@dsternlicht
Copy link
Author

@danielbarion there you go:
https://codesandbox.io/s/condescending-nightingale-dh3xe4?file=/src/App.js

In index.js if you try to change the root element to a simple div, the tooltip works.

@danielbarion
Copy link
Member

@dsternlicht thanks, we will take a look as soon as possible.

@gabrieljablonski
Copy link
Member

gabrieljablonski commented May 25, 2023

A few links I've skimmed through which have some interesting information:

https://www.wpeform.io/blog/render-react-app-shadow-dom-styled-components/
https://medium.com/rate-engineering/winning-the-war-of-css-conflicts-through-the-shadow-dom-de6c797b5cba
https://www.npmjs.com/package/react-shadow-dom-retarget-events

In summary, when using a shadow DOM, stuff inside it is pretty much isolated from the rest of the DOM, so doing stuff to components inside it (applying CSS, listening to events) is not transparent.

It doesn't seem like it would be too difficult to support it, but it's also not a trivial change.

Also not sure if completely relevant, but #636 seemed to deal with a similar problem in v4.

@dsternlicht
Copy link
Author

@gabrieljablonski I believe that shadow DOM is pretty common nowadays, and it's worth supporting it if possible.

@danielbarion Can this be handled the same way it did in v4?

@danielbarion
Copy link
Member

@dsternlicht I'll take a look as soon as I have time on your example and I'll let you know

@dsternlicht
Copy link
Author

Thanks @danielbarion. If you can direct me to the right place I can try figure it out myself and PR it 🙂

@danielbarion
Copy link
Member

Today we are injecting the styles through the rollup config, we will need to check if postcss has some support to shadow DOM injection or we will need to disable this option and inject the styles manually

I'm thinking in those 2 ways to try solve the issue with the shadow DOM

@dsternlicht
Copy link
Author

@danielbarion injecting the styles manually will probably be much easier than interacting with rollup's config.

@danielbarion
Copy link
Member

I tried the manual injection today but I was not able to inject the styles (CSS Module) as text into the style element inside shadow dom, I'll try to take a look more deeply when I have more time, but PRs are always welcome! 🙂

@dsternlicht if you have time, try to implement the style injection manually and make it work, don't worry about the best implementation, try to make it work as expected without breaking anything and we can help you to improve the code if needed.

@dsternlicht
Copy link
Author

Thanks @danielbarion can you just direct me to the correct file that should handle it?

@danielbarion
Copy link
Member

danielbarion commented Jun 5, 2023

@dsternlicht I'm sorry for the delay, we probably will need to handle it in this file (Tooltip.tsx) because this is the place that we have the rendered node information

@danielbarion
Copy link
Member

Guys, I tried in my available time today but I need help, here is the draft PR: #1039

cc @dsternlicht @gabrieljablonski

@dsternlicht
Copy link
Author

dsternlicht commented Jun 13, 2023

@danielbarion Thanks Daniel! I left a comment just to understand where you got stuck.

@danielbarion
Copy link
Member

I tried again but I was not able to make it work as expected on shadow DOM, so, I closed the PR and we are accepting new PRs related to this issue.

Thanks all!

@quentin-benyahia
Copy link

Hey @danielbarion @dsternlicht

I got some insight regarding this issue, we faced the same problems and wanted to make it work so I had a look in the code to see if I could do something and I was able to make some use cases work pretty well (especially on the hover part, but need help on the onClick stuff)

#1068

I was focusing on making it work so the code isn't exactly proper but I think it deserves improvement and a some help to make it work just fine.

Overall I'm stuck on making some use cases work, especially the onClick one, I think some "addEventListener" should be done on the Shadow Dom instead of the window in the TooltipController.tsx but I wasn't able to fully dig into every part of the code and my tries to make it work were unsuccessful today (even with dirty code)

I will have some extra look this week, but if you guys think about anything lmk !

Thanks

@danielbarion
Copy link
Member

@quentin-benyahia thanks for the PR, we will take a look as soon as possible.
cc @gabrieljablonski

@dsternlicht
Copy link
Author

@quentin-benyahia very cool!

@danielbarion can this be merged (after a code review) to at least fix the hover behaviour on Shadow DOM?

@quentin-benyahia
Copy link

It's more of a POC for now, but I think overall I'll follow the same idea, I'll wait for a review just to make sure the ideas make sense, cause I was thinking that maybe a few parts could be done differently, then make it prod ready (remove useless comments, factorize some stuff ..etc)

Thanks !

@gabrieljablonski gabrieljablonski added the V6 It might get fixed/merged before, but most likely only on V6's release. label Aug 2, 2023
@dsternlicht
Copy link
Author

Any news here? :(

@quentin-benyahia
Copy link

quentin-benyahia commented Oct 25, 2023

@dsternlicht Sorry, I thought I updated this conv but it seems like I've only commented on the PR (#1068), I wasn't able to make the proper fix on a PR because I lack of time on this topic, I had to fork and adapt the package to make it work in our use cases only.

@dsternlicht
Copy link
Author

@dsternlicht Sorry, I thought I updated this conv but it seems like I've only commented on the PR (#1068), I wasn't able to make the proper fix on a PR because I lack of time on this topic, I had to fork and adapt the package to make it work in our use cases only.

Can you share a link to the fork and the fix?

@danielbarion
Copy link
Member

danielbarion commented Oct 25, 2023

Hi guys,

@quentin-benyahia if you can, please create one PR for us with the working code on shadow DOM, or create one repository with it so we can test and help adapt it to the project.

Any questions or need help with the PR, we will try to dive into it and help fix the broken features, we need the basic tooltip working to start helping on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Help Wanted V6 It might get fixed/merged before, but most likely only on V6's release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants