Skip to content

Resolve YouTube Error 153 and add graceful degradation in JourneyModal#1687

Open
PARTH-TUSSLE wants to merge 2 commits into
layer5io:masterfrom
PARTH-TUSSLE:fix/getting-started-video-fallback
Open

Resolve YouTube Error 153 and add graceful degradation in JourneyModal#1687
PARTH-TUSSLE wants to merge 2 commits into
layer5io:masterfrom
PARTH-TUSSLE:fix/getting-started-video-fallback

Conversation

@PARTH-TUSSLE

@PARTH-TUSSLE PARTH-TUSSLE commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Notes for Reviewers

Description

This PR addresses Error 153: Video player configuration error" that occurs in the Getting Started widget's YouTube iframe when viewed on browsers with strict privacy settings (e.g if someone's using Brave browser etc).

It fixes the configuration error natively, and adds a gracefully styled fallback link for users whose networks or aggressive ad-blockers block the youtube.com domain entirely.

Changes Made

  • Added referrerPolicy="strict-origin-when-cross-origin" to the YouTube iframe to ensure required origin headers are sent, natively fixing Error 153.
  • Added a FallbackLink styled component below the iframe to allow users to watch the video directly on YouTube if the embed is blocked by network-level firewalls or ad-blockers.
  • Styled the fallback anchor tag using theme.palette.text.brand to ensure high contrast and visibility in both Light and Dark modes.
{89290EE3-5D32-49DD-908F-7210E80F06BB}

Related issue -> meshery/meshery#20118
Related PR -> meshery/meshery#20520

Signed commits

  • [✅] Yes, I signed my commits.

Signed-off-by: Parth Gartan <parthgartan26feb@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request adds a fallback link below the embedded YouTube video iframe in the JourneyModal component to allow users to watch the video directly on YouTube if they encounter issues, and updates the iframe with a strict referrer policy. The review feedback recommends extracting the hardcoded YouTube video ID into a constant to prevent duplication and exposing the fallback link text as a configurable prop to support internationalization and localization.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +41 to +53
const FallbackLink = styled('div')(({ theme }) => ({
textAlign: 'center',
marginTop: '0.5rem',
marginBottom: '1rem',
'& a': {
color: theme.palette.text.brand,
textDecoration: 'none',
fontWeight: 'bold',
'&:hover': {
textDecoration: 'underline'
}
}
}));

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.

medium

To support internationalization (i18n) and localization, avoid hardcoding user-facing UI strings in shared components. Instead, expose these strings as configurable props with default values to maintain backward compatibility.

We should expose the fallback text as a prop (e.g., 'fallbackText') with a default value of '▶ Having trouble viewing? Watch on YouTube'. Additionally, we can extract the YouTube video ID to a constant to avoid duplication.

const YOUTUBE_VIDEO_ID = 'Do7htKrRzDA';

const FallbackLink = styled('div')(({ theme }) => ({
  textAlign: 'center',
  marginTop: '0.5rem',
  marginBottom: '1rem',
  '& a': {
    color: theme.palette.text.brand,
    textDecoration: 'none',
    fontWeight: 'bold',
    '&:hover': {
      textDecoration: 'underline'
    }
  }
}));
References
  1. Avoid hardcoding UI strings (such as button labels) in shared components. Expose these strings as configurable props to support internationalization (i18n) and localization.
  2. When exposing UI strings as configurable props in shared components to support localization, provide a default value to maintain backward compatibility.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

resolved

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Stop being lazy, @PARTH-TUSSLE. "resolved" as a response to the feedback on your PR isn't going to cut it.
Please reply to each review comment. Either show how you incorporated the feedback or explain why you're rejecting it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have updated the PR to extract the video ID into a YOUTUBE_VIDEO_ID constant to prevent duplication. I've also added an optional fallbackText prop to the JourneyModalProps interface with the default value of '▶ Having trouble viewing? Watch on YouTube', which will properly support localization without breaking backward compatibility. These changes have been pushed!"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@leecalcote , I'll keep in mind replying to bot's suggestion properly from now on.

Comment on lines +165 to +184
<iframe
style={{
aspectRatio: '16/9',
width: '100%'
}}
src="https://www.youtube.com/embed/Do7htKrRzDA?si=5iMQ5a1JUf3qpIiH"
title="YouTube video player"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share"
allowFullScreen={true}
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
<FallbackLink>
<a
href="https://www.youtube.com/watch?v=Do7htKrRzDA"
target="_blank"
rel="noopener noreferrer"
>
▶ Having trouble viewing? Watch on YouTube
</a>
</FallbackLink>

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.

medium

Use the newly defined 'YOUTUBE_VIDEO_ID' constant and the 'fallbackText' prop here to avoid duplicating the hardcoded video ID and to support localization.

Suggested change
<iframe
style={{
aspectRatio: '16/9',
width: '100%'
}}
src="https://www.youtube.com/embed/Do7htKrRzDA?si=5iMQ5a1JUf3qpIiH"
title="YouTube video player"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share"
allowFullScreen={true}
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
<FallbackLink>
<a
href="https://www.youtube.com/watch?v=Do7htKrRzDA"
target="_blank"
rel="noopener noreferrer"
>
Having trouble viewing? Watch on YouTube
</a>
</FallbackLink>
<iframe
style={{
aspectRatio: '16/9',
width: '100%'
}}
src={"https://www.youtube.com/embed/" + YOUTUBE_VIDEO_ID + "?si=5iMQ5a1JUf3qpIiH"}
title="YouTube video player"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share"
allowFullScreen={true}
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
<FallbackLink>
<a
href={"https://www.youtube.com/watch?v=" + YOUTUBE_VIDEO_ID}
target="_blank"
rel="noopener noreferrer"
>
{fallbackText}
</a>
</FallbackLink>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

resolved

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Incorporated! I've updated the JSX block to dynamically get the new YOUTUBE_VIDEO_ID constant into both the iframe src and the fallback link's href. I also replaced the hardcoded string inside the anchor tag with the new {fallbackText} prop as suggested.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've previously given feedback that this isn't desired.

Nor is the hardcoding of a specific video id into a "reusable" component in our design system.

Signed-off-by: Parth Gartan <parthgartan26feb@gmail.com>
@PARTH-TUSSLE

Copy link
Copy Markdown
Contributor Author

@leecalcote leecalcote left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where is the link to your other PR and the feedback already received?

@PARTH-TUSSLE

Copy link
Copy Markdown
Contributor Author

Where is the link to your other PR and the feedback already received?

@leecalcote I've updated the PR description with the open PR in meshery regarding this issue. And I actually closed the other PR ( the one which I was working on earlier ) because it had many commits now cause I was working on it for 2 weeks, and to start fresh from synced masters, I created new branches and PRs . Should I link the closed PR as well, in case someone wants to see the earlier reviews?

@leecalcote

Copy link
Copy Markdown
Member

This widget shouldn't be included in Sistent in the first place.

@PARTH-TUSSLE

Copy link
Copy Markdown
Contributor Author

This widget shouldn't be included in Sistent in the first place.

@leecalcote Agreed, sistent should only have reusable componenets. I was only making these changes here to fix the YouTube Error 153 on strict browsers bug , where the code currently lives. Along with this Getting started widget, there are several other specific widgets in Sistent right now (e.g., RecentDesignWidget, WorkspaceActivityWidget, TeamSearchField etc.). Should we now migrate the GettingStartedWidget (and potentially the other Dashboard widgets) directly into the meshery repository? Let me know how to proceed.

@PARTH-TUSSLE PARTH-TUSSLE requested a review from leecalcote July 7, 2026 09:54
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