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

feat #5246: add styleContainer option for PrimeReactContext #5566

Merged
merged 3 commits into from
Jan 5, 2024

Conversation

mondaychen
Copy link
Contributor

@mondaychen mondaychen commented Dec 10, 2023

Fix #5246
Fix #4096

As discussed in https://github.com/orgs/primefaces/discussions/630#discussion-5931944, this is to add styleContainer in PrimeReactContext so that developers can configure useStyle to insert into a specific container. This is needed when building the app within a shadow dom.

Example use:

root.attachShadow({ mode: 'open' }); // Open the shadowRoot
const mountHere = root.shadowRoot;

const options = { appendTo: mountHere, styleContainer: mountHere};

ReactDOM.createRoot(mountHere).render(
  <React.StrictMode>
    <PrimeReactProvider value={options}>
      <App />
    </PrimeReactProvider>
  </React.StrictMode>
);

To seem it working:

  1. Clone https://github.com/mondaychen/prime-react-style-container-repro/
  2. run npm install && npm run dev

(Note this is the same as the reproducer in #5246 except use of new option styleContainer is added, but I had to make a repo because on stackblitz.com I was not able to point to github in package.json like https://github.com/mondaychen/prime-react-style-container-repro/blob/main/package.json#L13 (git command is unavailable in terminal)

Before (Dropdown style broken):

image

After:
image

Copy link

vercel bot commented Dec 10, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Jan 5, 2024 9:55pm
primereact-v9 ⬜️ Ignored (Inspect) Visit Preview Jan 5, 2024 9:55pm

@mondaychen mondaychen changed the title feat #5246: add styleContainer option for useStyle feat #5246: add styleContainer option for PrimeReactContext Dec 10, 2023
@melloware melloware added the Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add label Dec 10, 2023
@melloware
Copy link
Member

melloware commented Dec 10, 2023

Can you check the code base there are other places where styles are added using DomHandler.createInlineStyle() and removeInlineStyle() which I assume would have the same problem?

@mondaychen
Copy link
Contributor Author

@melloware ah nice catch.
How about we add an optional 2nd parameter as styleContainer for createInlineStyle? So the usage is changed to

DomHandler.createInlineStyle(
  (context && context.nonce) || PrimeReact.nonce,
  context && context.styleContainer
);

For removeInlineStyle I think instead of document.head.removeChild(styleElement) we can just do styleElement.parentNode.removeChild(styleElement)

@melloware
Copy link
Member

Yep definitely pass a second param!

@melloware
Copy link
Member

Also don't forget to update the docs example here: https://primereact.org/configuration/

@mondaychen
Copy link
Contributor Author

Sorry I've been sick these days. Will get back to this once I'm recovered

@melloware
Copy link
Member

@mondaychen you still working on this?

@mondaychen
Copy link
Contributor Author

@melloware Yes I will continue to work on this once I'm back from holiday break. Is there a deadline or is it considered an urgent issue now?

@melloware
Copy link
Member

Awesome. No I was just checking in on you.

@mondaychen
Copy link
Contributor Author

@melloware updated DomHandler and added docs
image

@mondaychen
Copy link
Contributor Author

In the "AppendTo" section it says "By default all popups are append to document body via Portals.", so I used a similar sentence. But should we say "appended" instead?
I'm not native speaker so wanna confirm first

@SoyDiego
Copy link
Contributor

SoyDiego commented Jan 5, 2024

In the "AppendTo" section it says "By default all popups are append to document body via Portals.", so I used a similar sentence. But should we say "appended" instead?
I'm not native speaker so wanna confirm first

Maybe @melloware can help here

@melloware
Copy link
Member

melloware commented Jan 5, 2024

Yes it should be "appended" !

"By default all popups are appended to the document body via Portals.

@melloware
Copy link
Member

@mondaychen let me know when this is ready but it looks good to me!

@mondaychen
Copy link
Contributor Author

Updated the text & it's ready! @melloware

@melloware melloware merged commit 3afafe7 into primefaces:master Jan 5, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Projects
None yet
3 participants