fix: theme logic #448
fix: theme logic #448
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@luwes is attempting to deploy a commit to the Mux Team on Vercel. A member of the Team first needs to authorize it. |
What's that mean? |
| templateInstances.set(part, tpl); | ||
| } else { | ||
| templateInstances.get(part)?.update(state); | ||
| } |
There was a problem hiding this comment.
@heff this code is the gist of the template caching.
it's related to the inner templates (directives), they themselves are an extension of a child node part but instead of having a simple text node or element as value they have another template instance as contents.
when creating a new TemplateInstance (extension of DocumentFragment) basically what happens is the is cloned, the contents is appended to the TemplateInstance, it's parsed for parts, parts are created and stored in an array together with the string expressions, TemplateInstance.update(state) is called which calls the template processor and loops through the parts and updates the part's values.
tldr; when the template associated with the part did not change we can call templateInstance.update() directly without creating a new templateInstance and blowing away the previous html fragment.
| (next = cur.nextSibling), parent.removeChild(cur), (cur = next); | ||
| } | ||
|
|
||
| return b; |
There was a problem hiding this comment.
I put in this minimal node list differ because something was wrong with my remove/add logic and it left an element from a previous permutation in the DOM.
| ...Array.from(this.mediaController?.attributes ?? []) | ||
| .filter(({ name }) => { | ||
| return observedMediaAttributes[name] || name.startsWith('breakpoint-') | ||
| }) | ||
| }), | ||
| ...Array.from(this.attributes), |
There was a problem hiding this comment.
question(non-blocking): In other words, you want attributes to effectively function as overrides for any coming from media-controller?
There was a problem hiding this comment.
yes indeed, I needed this behavior in Mux player so when stream-type is set manually it takes effect instantly and overrides the one coming from Media Chrome
There was a problem hiding this comment.
Cool that sounds like the right contract/precedence imo.
| @@ -9,7 +9,6 @@ export * from './utils/template-parts.js'; | |||
| const observedMediaAttributes = { | |||
| 'media-target-live-window': 'targetLiveWindow', | |||
| 'media-stream-type': 'streamType', | |||
| 'audio': 'audio', | |||
There was a problem hiding this comment.
thought(non-blocking): We may want to eventually re-add this for convenience, though most (non-player) theme use cases will likely either be audio or video anyway, so this likely doesn't need to be a priority (also not a priority for Mux Player, given how we manage audio only).
| } | ||
|
|
||
| this.#nodes = normalisedNodes; | ||
| this.#nodes = swapdom( |
my bad, this is set manually on the theme element
luwes
force-pushed
the
fix-theme-logic
branch
from
c6b4801
to
6b8c67a
Compare
audiofrom template vars, this is set manually on the<media-theme>element so this was a mistake