muxinc / media-chrome

Custom elements (web components) for making audio and video player controls that look great in your website or app.
https://media-chrome.org
MIT License
1.77k stars 71 forks source link

fix: Since attributeChangedCallback can be invoked before DOM connect… #742

Closed cjpillsbury closed 1 year ago

cjpillsbury commented 1 year ago

…ion, only attempt to resolve media controller id if isConnected.

fixes #741

vercel[bot] commented 1 year ago

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

Name Status Preview Comments Updated (UTC)
media-chrome ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 9, 2023 8:46pm
media-chrome-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 9, 2023 8:46pm
codecov[bot] commented 1 year ago

Codecov Report

Attention: 52 lines in your changes are missing coverage. Please review.

Comparison is base (bbf310f) 79.35% compared to head (28092f4) 79.25%. Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #742 +/- ## ========================================== - Coverage 79.35% 79.25% -0.11% ========================================== Files 56 57 +1 Lines 10236 10300 +64 ========================================== + Hits 8123 8163 +40 - Misses 2113 2137 +24 ``` | [Files](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc) | Coverage Δ | | |---|---|---| | [src/js/media-chrome-button.js](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc#diff-c3JjL2pzL21lZGlhLWNocm9tZS1idXR0b24uanM=) | `89.86% <100.00%> (-0.45%)` | :arrow_down: | | [src/js/media-chrome-range.js](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc#diff-c3JjL2pzL21lZGlhLWNocm9tZS1yYW5nZS5qcw==) | `92.92% <100.00%> (ø)` | | | [src/js/media-gesture-receiver.js](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc#diff-c3JjL2pzL21lZGlhLWdlc3R1cmUtcmVjZWl2ZXIuanM=) | `71.93% <100.00%> (ø)` | | | [src/js/media-loading-indicator.js](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc#diff-c3JjL2pzL21lZGlhLWxvYWRpbmctaW5kaWNhdG9yLmpz) | `91.70% <100.00%> (ø)` | | | [src/js/media-preview-thumbnail.js](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc#diff-c3JjL2pzL21lZGlhLXByZXZpZXctdGh1bWJuYWlsLmpz) | `63.29% <100.00%> (ø)` | | | [src/js/media-text-display.js](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc#diff-c3JjL2pzL21lZGlhLXRleHQtZGlzcGxheS5qcw==) | `97.60% <100.00%> (ø)` | | | [src/js/utils/server-safe-globals.js](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc#diff-c3JjL2pzL3V0aWxzL3NlcnZlci1zYWZlLWdsb2JhbHMuanM=) | `96.66% <100.00%> (+0.11%)` | :arrow_up: | | [src/js/experimental/media-chrome-listbox.js](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc#diff-c3JjL2pzL2V4cGVyaW1lbnRhbC9tZWRpYS1jaHJvbWUtbGlzdGJveC5qcw==) | `61.96% <92.30%> (+0.57%)` | :arrow_up: | | [src/js/media-control-bar.js](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc#diff-c3JjL2pzL21lZGlhLWNvbnRyb2wtYmFyLmpz) | `81.98% <0.00%> (ø)` | | | [src/js/utils/element-utils.js](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc#diff-c3JjL2pzL3V0aWxzL2VsZW1lbnQtdXRpbHMuanM=) | `88.13% <60.00%> (-1.69%)` | :arrow_down: | | ... and [2 more](https://app.codecov.io/gh/muxinc/media-chrome/pull/742?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=muxinc) | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cjpillsbury commented 1 year ago

approving but there is an edge case where someone could upgrade a custom element not in the document and the getElementById could work like intended. can follow up if needed

Not sure I follow? Are you concerned about a case that wouldn't get handled by our code in connectedCallback()?