Skip to content

Fix media-viewer: src reactivity, error state, configurable alt, CORS fallback, duplicate fetches#91

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-media-viewer-deficiencies
Draft

Fix media-viewer: src reactivity, error state, configurable alt, CORS fallback, duplicate fetches#91
Copilot wants to merge 2 commits intomainfrom
copilot/fix-media-viewer-deficiencies

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 17, 2026

<media-viewer> had several deficiencies: no re-fetch when src changed after mount, blank UI on fetch failure, hardcoded alt="Image", silent CORS failures with no fallback, and duplicate HEAD requests on reconnection.

Changes

  • src reactivity: Added updated() hook that resets mimeType/_error and re-fetches when src changes.
  • Error state: Added @state() _error — renders <div class="error">Unable to load media</div> instead of a permanently blank loading div on fetch failure.
  • Configurable alt: Added @property() alt = 'Image' passed through to <img alt=${this.alt}>.
  • CORS/missing Content-Type fallback: Extracted fallbackToExtensionBasedType() — infers video/{ext} for mp4/webm/ogg, falls back to image/unknown otherwise. Called in both the no-header and catch paths.
  • Duplicate fetch guard: Added _fetchingFor: string | null flag — set on fetch start, cleared in finally — prevents the double HEAD request that occurs when connectedCallback and updated both fire on initial mount with a src attribute set.
// src changes are now handled reactively
override updated(changedProperties: PropertyValues) {
  if (changedProperties.has('src') && this.src && this._fetchingFor !== this.src) {
    this.mimeType = null;
    this._error = false;
    this.determineFileType();
  }
}

// CORS/network failures now fall back to extension-based type detection
private fallbackToExtensionBasedType() {
  const ext = this.src?.split('.').pop()?.toLowerCase();
  if (ext && ['mp4', 'webm', 'ogg'].includes(ext)) {
    this.mimeType = `video/${ext}`;
  } else {
    this.mimeType = 'image/unknown';
  }
}

Added .error CSS class to media-viewer-styles.ts to match .unsupported styling.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Feature][Medium] Fix media-viewer component deficiencies: src change handling, error state, alt text, CORS fallback</issue_title>
<issue_description>## Summary

The <media-viewer> component has several functional deficiencies that impact reliability and accessibility. These are grouped together as they all affect the same component.

Findings

1. No re-fetch on src property change

File: src/media-viewer/media-viewer.ts, lines 19-22

determineFileType() is only called in connectedCallback(). If the src property changes after the component is connected to the DOM, the MIME type is never re-determined and the component renders stale content.

Fix: Add a updated() lifecycle hook that watches for src changes:

override updated(changedProperties: PropertyValues) {
  if (changedProperties.has('src') && this.src) {
    this.mimeType = null;
    this.determineFileType();
  }
}

2. Blank error state — no visual feedback on failure

File: src/media-viewer/media-viewer.ts, lines 58-65

When the HEAD fetch() fails (network error, CORS restriction), mimeType stays null and the component renders <div class="loading"></div>. The .loading CSS class has no visible content — users see a blank space permanently.

Fix: Add an error state property and render meaningful fallback content:

@state() private _error = false;

// In render():
if (this._error) {
  return html`<div class="error">Unable to load media</div>`;
}

3. Hardcoded alt="Image" — not configurable

File: src/media-viewer/media-viewer.ts, line 70

The <img> element has alt="Image" hardcoded. Consumers cannot provide meaningful alt text for accessibility.

Fix: Add an alt property:

@property() alt = 'Image';
// Use in render:
html`<img src=${this.src} alt=${this.alt} ... />`

4. CORS dependency without fallback

File: src/media-viewer/media-viewer.ts, lines 24-36

The fetch(this.src, { method: 'HEAD' }) call fails silently on CORS-restricted URLs. The component should fall back to file extension detection when the HEAD request fails:

catch (error) {
  // Fallback: determine type from file extension
  const ext = this.src?.split('.').pop()?.toLowerCase();
  if (['mp4', 'webm', 'ogg'].includes(ext || '')) {
    this.mimeType = `video/${ext}`;
  } else {
    this.mimeType = 'image/unknown'; // Default to image rendering
  }
}

5. Duplicate HEAD requests on reconnection

File: src/media-viewer/media-viewer.ts, line 20

connectedCallback() always calls determineFileType() even if mimeType is already set. Add a guard:

override connectedCallback() {
  super.connectedCallback();
  if (!this.mimeType) {
    this.determineFileType();
  }
}
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…S fallback, duplicate requests

Co-authored-by: numbers-official <181934381+numbers-official@users.noreply.github.com>
Copilot AI changed the title [WIP] [Feature] Fix media-viewer component deficiencies Fix media-viewer: src reactivity, error state, configurable alt, CORS fallback, duplicate fetches Mar 17, 2026
Copilot AI requested a review from numbers-official March 17, 2026 13:19
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

Successfully merging this pull request may close these issues.

[Feature][Medium] Fix media-viewer component deficiencies: src change handling, error state, alt text, CORS fallback

2 participants