GoogleChrome / dialog-polyfill

Polyfill for the HTML dialog element
BSD 3-Clause "New" or "Revised" License
2.45k stars 245 forks source link

automatically inject css into head #138 #180

Open mreinstein opened 5 years ago

mreinstein commented 5 years ago

this removes the need for manually inserting the css into <head>

akc42 commented 5 years ago

I think there could be a problem with this. I only just started tested using the polyfill today using Safari - all previous uses have been with Chrome, but...

I have <dialog> elements inside custom elements, and I have found that I don't want the css file imported into <head>. Instead it has to be imported into the shadowRoot of the custom element that uses it.

mreinstein commented 5 years ago

@akc42 valuable use case, thanks for sharing! We could potentially modify the registerDialog function to accept some optional configuration around where to inject the style:

registerDialog(element, { cssParent: element.shadowRoot })

If the 2nd argument is missing or cssParent is invalid, keep defaulting to head.

open for suggestions!

akc42 commented 5 years ago

I think you can actually work out automatically whether to inject into the shadowRoot or the document header. The branch of the polyfill I am actually using now is one cloned from the one made to support custom elements - see #168 and that finds out if there is a shadowRoot or whether to use the document.body

I am using contructable style sheets to do the insertion (see https://github.com/WICG/construct-stylesheets/blob/gh-pages/explainer.md) for what these are (see also npm package construct-style-sheets-polyfill). Note I am using lit-element which already has an ability to use this facility without the npm package.

For the efficiency reasons stated in this proposal, I guess this is also the approach you should take if you continue to do it automatically. However there is a downside as it appears that they are injected below the styles that are already there and so take precedence and this caused me problems - see below

So I also ended up not including some of the css (the piece just styling dialog) because I had already styled it differently in my different custom elements. In particular, in one particular case I was moving the dialog to sit just under where the user just clicked on the page (itself scaled with a css transform) and was using getBoundingClientRect() to workout the dialog size and do a calculation to add a style of top and left to the element. I had broken this code and the positioning was way off. I eventually found out that was because of margin:auto in the css, whereas I had explicitly tried to style it with margin:0 and that had not overridden.

(I think I took the simpler approach of removing the styling for the dialog css - maybe a better approach would have been to make an additional constructable style sheet for my styling and added that after the polyfill one so it overrode it)