fix(jss): Wrong sheet order when the insertion point is a DOM element. #816
Conversation
| @@ -263,7 +263,7 @@ function insertStyle(style: HTMLElement, options: PriorityOptions) { | |||
| // https://stackoverflow.com/questions/41328728/force-casting-in-flow | |||
| const insertionPointElement: HTMLElement = (insertionPoint: any) | |||
| const {parentNode} = insertionPointElement | |||
| if (parentNode) parentNode.insertBefore(style, insertionPointElement.nextSibling) | |||
| if (parentNode) parentNode.insertBefore(style, prevNode) | |||
There was a problem hiding this comment.
prevNode will always be null here.
There was a problem hiding this comment.
Indeed it will, as will it in the "getHead().insertBefore" line below. Perhaps using appendChild for both is more descriptive?
|
I did some digging, and I don't think this is the place where the bug is. The problem is that That's the problem which is causing the displacement of the style tag. |
|
Your change is going to cause issues when the first style tag is attached, and there is an element after the insertion point. This would cause it to be inserted last at the parent! |
|
ok i'll look at adding a few more test cases and a different fix over the next week. |
|
Your current fix should be reverted. Also |
|
Solved by #819 |
When a new sheet is added with an index higher than all of those that already exist and the insertion point is an element the style sheets are actually added to the top of the list of sheets instead of the bottom.
The logic for inserting into the head vs inserting into a different element should be the same but currently isn't.
The modified test fails without the change and passes after.
This bug is particularly impactful when using the ShadowDOM as styles need to be added to an element within the shadow tree instead of the head and currently prevents using material-ui components inside a shadow root.
Fixes #752