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

Prevent page styles from bleeding into the component #1274

Open
rameshanandakrishnan opened this issue Nov 1, 2021 · 3 comments
Open

Prevent page styles from bleeding into the component #1274

rameshanandakrishnan opened this issue Nov 1, 2021 · 3 comments

Comments

@rameshanandakrishnan
Copy link

Currently, OC components are within the scope of the light DOM and do not provide any forms of style isolation out of the box. This potentially leaves the components to be altered by rogue global styles. Style isolation will be a nice feature to have so that we can ensure that the component will look the same regardless of where you use it. I would like to propose the inclusion of shadow DOM as part of the OC to prevent page styles from affecting the component and also prevent the component styles from leaking out.

I have played around with a POC and this is how it can be achieved:

  1. The means of toggling the shadow DOM feature can be handled via data-attributes (e.g. <oc-component …. data-enableShadow=true … /> ) of the OC element.
  2. We will need to update oc-clint.js in OC-CLIENT-BROWSER to handle the creation of shadow root in which the HTML markup can be set. This can be done in the processHTML method.
  3. Update the relevant templates to indicate that the target node is currently present in the shadow root rather than the light DOM.

I would like your feedback on this matter and am happy to make a PR once we agree on a solution.

@ricardo-devis-agullo
Copy link
Collaborator

What do you think, @matteofigus ?

@matteofigus
Copy link
Member

Hard to say as I think we need more people that use OC to give us feedback.

Generally speaking, I think it makes sense for a client-side rendered component to have shadow dom created as default. For server-side, not sure as I remember that the most common use case for SSR was for rendering things like Header+Footer and all would inherit from global styles. But as far as we make it optional with sensible defaults, I think I'm onboard.

I think I have some ideas about the implementation. Bringing the code inside the client is tempting but makes it harder for people to customise and optimise. I think that the original vision for OC base libraries/clients VS templates was to actually allow this sort of customisations. Maybe if we want to stay consistent with that vision (but I'm open to change my mind if we disagree as a group), we should bring the shadow DOM creation inside the "ES6" template client wrapper and keep the base browser client "dumb" - this would allow people that have different templates not to be affected, but also new customers working on the base template to just easily take advantage of it.

What do you think?

@rameshanandakrishnan
Copy link
Author

@matteofigus Thanks for the feedback. I am not sure if this could be just done without editing the client browser at all. Currently, the browser client is responsible for injecting the HTML using innerHTML, and I do not think we will be able to capture shadow DOM as a HTML string. So a solution that is purely template-based might not be possible.

At a template level, we could create an HTML node with a shadow DOM and append the HTML string to the shadow root before asking the browser client to render the content. If we go down this path, we will still need to update the client browser to use the element.append() instead of element.innerHTML(). This will be a breaking change for all the other templates so this option is not worth entertaining.

The only way I can think of this being 100% on the template is by wrapping the content in a template element so that it does not get rendered right away. Then create a custom element that will eventually pick up the content within the template and render itself once it has loaded. Not sure if it will work and the solution will be a little complicated but it is worth trying.

I am happy to hear different suggestions on how this can be achieved.

A pro of having the changes on the client browser level is that we would make the changes in a single place and could potentially support this feature for all the other templates. React would be the odd one out since we depend on the reactdom.render method to render the component. So react based templates will require some additional work at a template level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants