Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PARTH-TUSSLE wants to merge 2 commits into
base: master
Choose a base branch
from
+41 −11
Open

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23e2d15
Resolve YouTube Error 153 and add graceful degradation in JourneyModal
PARTH-TUSSLE Jul 7, 2026
41e3354
Add fallbackText prop for i18n and extract YOUTUBE_VIDEO_ID constant
PARTH-TUSSLE Jul 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 41 additions & 11 deletions src/custom/DashboardWidgets/GettingStartedWidget/JourneyModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ interface JourneyModalProps {
};
profileData?: ProfileData;
embedDesignPath: string;
fallbackText?: string;
}

const YOUTUBE_VIDEO_ID = 'Do7htKrRzDA';

const stepperType = {
Previous: 'Previous',
Next: 'Next'
Expand All @@ -38,6 +41,20 @@ export const ModalVideo = styled('video')(() => ({
paddingBlock: '1rem'
}));

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'
}
}
}));
Comment on lines +44 to +56

@gemini-code-assist gemini-code-assist Bot Jul 7, 2026

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.

@PARTH-TUSSLE PARTH-TUSSLE Jul 7, 2026

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

@leecalcote leecalcote Jul 7, 2026

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.

@PARTH-TUSSLE PARTH-TUSSLE Jul 7, 2026

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!"

@PARTH-TUSSLE PARTH-TUSSLE Jul 7, 2026

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.


const JourneyModal: React.FC<JourneyModalProps> = ({
open,
handleClose,
Expand All @@ -46,7 +63,8 @@ const JourneyModal: React.FC<JourneyModalProps> = ({
stepsData,
useNotificationHandlers,
profileData,
embedDesignPath
embedDesignPath,
fallbackText = '▶ Having trouble viewing? Watch on YouTube'
}) => {
const [currentModal, setCurrentModal] = useState<number>(0);
const [step, setStep] = useState<StepData | null>(null);
Expand Down Expand Up @@ -147,16 +165,28 @@ const JourneyModal: React.FC<JourneyModalProps> = ({
''
)}
{data.video !== undefined ? (
<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}
></iframe>
<>
<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>
Comment on lines +169 to +188

@gemini-code-assist gemini-code-assist Bot Jul 7, 2026

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>

@PARTH-TUSSLE PARTH-TUSSLE Jul 7, 2026

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

@PARTH-TUSSLE PARTH-TUSSLE Jul 7, 2026

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.

@leecalcote leecalcote Jul 7, 2026

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.

</>
) : (
''
)}
Expand Down
Loading