Skip to content

fix: improve error message display#1042

Merged
tsi merged 2 commits intomasterfrom
VIDEO-20800-redesign-error-messages
May 5, 2026
Merged

fix: improve error message display#1042
tsi merged 2 commits intomasterfrom
VIDEO-20800-redesign-error-messages

Conversation

@tsi
Copy link
Copy Markdown
Collaborator

@tsi tsi commented May 3, 2026

Summary

Improves the Cloudinary Video Player error overlay so errors render as a centered, themed message instead of the previous flat gray block. Adds a dedicated title, message area, and retry action wired to the existing refresh event.

Changes

  • Adds structured error display content with a Something went wrong heading and localized error message body.
  • Adds a Try Again action when the error message does not already provide a .cld-error-refresh link.
  • Updates error overlay styling to use the player theme colors, centered layout, a muted poster backdrop, and a compact error panel.

Visuals

UI styling changed for player error states. Please attach before/after screenshots or verify the error overlay visually before review.

Before:

image

After:

image

@tsi tsi requested a review from a team as a code owner May 3, 2026 15:16
@netlify
Copy link
Copy Markdown

netlify Bot commented May 3, 2026

Deploy Preview for cld-video-player ready!

Name Link
🔨 Latest commit f036cdd
🔍 Latest deploy log https://app.netlify.com/projects/cld-video-player/deploys/69f7691234719d0008f0474b
😎 Deploy Preview https://deploy-preview-1042--cld-video-player.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 3, 2026

Deploy Preview for cld-vp-esm-pages ready!

Name Link
🔨 Latest commit f036cdd
🔍 Latest deploy log https://app.netlify.com/projects/cld-vp-esm-pages/deploys/69f76912185dc30008d50b6a
😎 Deploy Preview https://deploy-preview-1042--cld-vp-esm-pages.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown
Contributor

@adimiz1 adimiz1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test added for new header/refresh-conditional behavior, maybe it's worth adding one?

const wrapper = videojs.dom.createEl('div', { className: 'cld-error-display-content' });
const msg = videojs.dom.createEl('span', { className: 'cld-error-message' });
const header = videojs.dom.createEl('div', { className: 'cld-error-header' });
const title = videojs.dom.createEl('span', { className: 'cld-error-title' }, {}, this.localize('Something went wrong'));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something went wrong - should be hardcoded?

Copy link
Copy Markdown
Collaborator Author

@tsi tsi May 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted some generic title, so a default one must be hard-coded, but we can allow it to be configured I guess

const wrapper = videojs.dom.createEl('div', { className: 'cld-error-display-content' });
const msg = videojs.dom.createEl('span', { className: 'cld-error-message' });
const header = videojs.dom.createEl('div', { className: 'cld-error-header' });
const title = videojs.dom.createEl('span', { className: 'cld-error-title' }, {}, this.localize('Something went wrong'));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is cld-error-title needed here? I could not find it in code

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdym? we give this class to the "Something went wrong" span

@tsi tsi merged commit da1551f into master May 5, 2026
10 checks passed
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.

2 participants