refactor: template customisation screens design#8785
Merged
Conversation
Contributor
WalkthroughUI and copy updates across TemplateCustomization multi-step screens: responsive Next button width, Cards/TemplateCard preview replacing SocialImage, Card-based social preview UI, Done screen copy/navigation changes, LanguageScreen prop addition, localized string updates, and corresponding test adjustments. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 450ed60
☁️ Nx Cloud last updated this comment at |
github-actions
Bot
requested a deployment
to
Preview - journeys-admin
github-actions
Bot
temporarily deployed
to
Preview - journeys-admin
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx (1)
72-131:⚠️ Potential issue | 🔴 CriticalFix upload error handling to avoid false success + stuck loading state.
Line 76 sets loading to true, but failures outside the inner
try/finallypath can leave loading active. Also, Line 126 always shows a success snackbar even after errors.🛠️ Suggested fix
async function handleImageChange( file: File | null | undefined ): Promise<void> { if (journey == null || file == null) return setLoading(true) - const { data } = await createCloudflareUploadByFile({}) - if (data?.createCloudflareUploadByFile?.uploadUrl != null) { - const formData = new FormData() - formData.append('file', file) - try { - const response = await ( - await fetch(data?.createCloudflareUploadByFile?.uploadUrl, { - method: 'POST', - body: formData as unknown as FormDataType - }) - ).json() + try { + const { data } = await createCloudflareUploadByFile({}) + const uploadUrl = data?.createCloudflareUploadByFile?.uploadUrl + if (uploadUrl == null) { + throw new Error('Missing upload URL') + } + + const formData = new FormData() + formData.append('file', file) + const response = await ( + await fetch(uploadUrl, { + method: 'POST', + body: formData as unknown as FormDataType + }) + ).json() + + const imageId = response?.result?.id as string | undefined + if (imageId == null) throw new Error('Missing uploaded image id') - const src = `https://imagedelivery.net/${ - process.env.NEXT_PUBLIC_CLOUDFLARE_UPLOAD_KEY ?? '' - }/${response.result.id as string}/public` + const src = `https://imagedelivery.net/${ + process.env.NEXT_PUBLIC_CLOUDFLARE_UPLOAD_KEY ?? '' + }/${imageId}/public` - if (journey?.primaryImageBlock != null) { - await journeyImageBlockUpdate({ - variables: { - id: journey.primaryImageBlock.id, - journeyId: journey.id, - input: { src, alt: 'journey image' } - } - }) - } else { - const { data: imageData } = await journeyImageBlockCreate({ - variables: { - input: { journeyId: journey.id, src, alt: 'journey image' } - } - }) - if (imageData?.imageBlockCreate != null) { - await journeyImageBlockAssociationUpdate({ - variables: { - id: journey.id, - input: { primaryImageBlockId: imageData.imageBlockCreate.id } - } - }) - } - } - } catch (error) { - enqueueSnackbar( - t('Failed to update social image, please try again later'), - { - variant: 'error', - preventDuplicate: true - } - ) - } finally { - setLoading(false) - enqueueSnackbar(t('Social image updated'), { - variant: 'success', - preventDuplicate: true - }) + if (journey.primaryImageBlock != null) { + await journeyImageBlockUpdate({ + variables: { + id: journey.primaryImageBlock.id, + journeyId: journey.id, + input: { src, alt: 'journey image' } + } + }) + } else { + const { data: imageData } = await journeyImageBlockCreate({ + variables: { + input: { journeyId: journey.id, src, alt: 'journey image' } + } + }) + if (imageData?.imageBlockCreate != null) { + await journeyImageBlockAssociationUpdate({ + variables: { + id: journey.id, + input: { primaryImageBlockId: imageData.imageBlockCreate.id } + } + }) } } + + enqueueSnackbar(t('Social image updated'), { + variant: 'success', + preventDuplicate: true + }) + } catch (error) { + enqueueSnackbar(t('Failed to update social image, please try again later'), { + variant: 'error', + preventDuplicate: true + }) + } finally { + setLoading(false) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx` around lines 72 - 131, In handleImageChange: ensure loading is cleared on every exit and success snackbar is only shown on actual success by expanding the try/catch/finally scope to cover the entire upload/update flow (including the createCloudflareUploadByFile call and the branch where uploadUrl is missing); specifically, keep setLoading(true) at start, wrap the logic from createCloudflareUploadByFile through journeyImageBlockAssociationUpdate/journeyImageBlockUpdate in a single try, show enqueueSnackbar(..., variant: 'success') only inside the successful path after updates complete, show the error snackbar inside catch, and call setLoading(false) in the finally so journeyImageBlockCreate, journeyImageBlockUpdate, journeyImageBlockAssociationUpdate, enqueueSnackbar, and setLoading are properly referenced and handled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsx`:
- Around line 33-35: The preview rendering assumes steps[0] always exists and
will crash when transformer(...) returns an empty array; in DoneScreen, guard
the preview by checking that the computed steps (const steps = transformer(...))
has length > 0 (or steps[0] is defined) before accessing steps[0]; if empty,
render a safe fallback (null or a placeholder) or skip preview logic so the
component doesn't attempt to read properties on undefined. Update DoneScreen's
render/path that uses steps[0] to early-return or conditional-render based on
steps.length and keep references to transformer, steps, TreeBlock<StepBlock>
intact.
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksScreen.tsx`:
- Around line 227-229: In LinksScreen (LinksScreen.tsx) update the desktop
helper copy passed to t(...) to remove the duplicated word so the string reads
"This contains buttons linking to external sites. Check them and update the
links below."; locate the t(...) call inside the component's render/return and
replace the duplicated "contains contains" with a single "contains" (ensure any
corresponding i18n key/update is made if the text is sourced from translation
files).
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx`:
- Around line 215-236: The IconButton wrapping the file input is icon-only and
lacks an accessible name; add an aria-label (e.g., aria-label="Upload social
image" or similar) to the IconButton so screen readers announce the control, and
also add a matching aria-label or aria-describedby on StyledInput (which
currently uses data-testid="SocialScreenSocialImageInput") to ensure the file
input is accessible; keep the onChange handler handleImageChange as-is and
ensure the label text clearly describes the action.
---
Outside diff comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx`:
- Around line 72-131: In handleImageChange: ensure loading is cleared on every
exit and success snackbar is only shown on actual success by expanding the
try/catch/finally scope to cover the entire upload/update flow (including the
createCloudflareUploadByFile call and the branch where uploadUrl is missing);
specifically, keep setLoading(true) at start, wrap the logic from
createCloudflareUploadByFile through
journeyImageBlockAssociationUpdate/journeyImageBlockUpdate in a single try, show
enqueueSnackbar(..., variant: 'success') only inside the successful path after
updates complete, show the error snackbar inside catch, and call
setLoading(false) in the finally so journeyImageBlockCreate,
journeyImageBlockUpdate, journeyImageBlockAssociationUpdate, enqueueSnackbar,
and setLoading are properly referenced and handled.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/CustomizeFlowNextButton/CustomizeFlowNextButton.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/TextScreen/TextScreen.tsx
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx (1)
75-130:⚠️ Potential issue | 🟠 MajorFix upload state and snackbar flow in
handleImageChange.Line [75] sets loading
true, but loading is reset only inside the innerfinally(Line [124]). If upload URL is missing (or the initial mutation throws), loading can get stuck. Also, success is always shown in Line [125], even after failure.Suggested fix
async function handleImageChange( file: File | null | undefined ): Promise<void> { if (journey == null || file == null) return setLoading(true) - - const { data } = await createCloudflareUploadByFile({}) - if (data?.createCloudflareUploadByFile?.uploadUrl != null) { + try { + const { data } = await createCloudflareUploadByFile({}) + const uploadUrl = data?.createCloudflareUploadByFile?.uploadUrl + if (uploadUrl == null) throw new Error('No upload URL') + const formData = new FormData() formData.append('file', file) - try { - const response = await ( - await fetch(data?.createCloudflareUploadByFile?.uploadUrl, { - method: 'POST', - body: formData as unknown as FormDataType - }) - ).json() - const src = `https://imagedelivery.net/${ - process.env.NEXT_PUBLIC_CLOUDFLARE_UPLOAD_KEY ?? '' - }/${response.result.id as string}/public` + const response = await ( + await fetch(uploadUrl, { + method: 'POST', + body: formData as unknown as FormDataType + }) + ).json() + const src = `https://imagedelivery.net/${ + process.env.NEXT_PUBLIC_CLOUDFLARE_UPLOAD_KEY ?? '' + }/${response.result.id as string}/public` @@ - } catch (error) { - enqueueSnackbar( - t('Failed to update social image, please try again later'), - { - variant: 'error', - preventDuplicate: true - } - ) - } finally { - setLoading(false) - enqueueSnackbar(t('Social image updated'), { - variant: 'success', - preventDuplicate: true - }) - } - } + enqueueSnackbar(t('Social image updated'), { + variant: 'success', + preventDuplicate: true + }) + } catch (error) { + enqueueSnackbar(t('Failed to update social image, please try again later'), { + variant: 'error', + preventDuplicate: true + }) + } finally { + setLoading(false) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx` around lines 75 - 130, The upload flow sets setLoading(true) but only resets it in the inner finally and always shows a success snackbar; wrap the entire upload+mutations in a top-level try/catch/finally so setLoading(false) always runs and move the success enqueueSnackbar into the success path (after all awaits succeed). Specifically, in the handler around createCloudflareUploadByFile, create a top-level try { const {data} = await createCloudflareUploadByFile(...); if (!data?.createCloudflareUploadByFile?.uploadUrl) throw new Error('Missing uploadUrl'); /* perform fetch and await journeyImageBlockUpdate/journeyImageBlockCreate/journeyImageBlockAssociationUpdate as now */ enqueueSnackbar(t('Social image updated'), {variant:'success',preventDuplicate:true}); } catch (error) { enqueueSnackbar(t('Failed to update social image, please try again later'), {variant:'error',preventDuplicate:true}); } finally { setLoading(false); } This ensures createCloudflareUploadByFile, fetch response, and GraphQL mutations (journeyImageBlockUpdate, journeyImageBlockCreate, journeyImageBlockAssociationUpdate) errors are handled, loading is cleared on every path, and success is only shown when all steps complete.
♻️ Duplicate comments (2)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsx (1)
33-35:⚠️ Potential issue | 🟠 MajorGuard preview rendering when no steps are returned.
Line [84] still passes
steps[0]directly. Iftransformer(...)returns an empty array, preview rendering can fail at runtime.Suggested fix
const steps = transformer(journey?.blocks ?? []) as Array< TreeBlock<StepBlock> > + const previewStep = steps[0] @@ - <TemplateCardPreviewItem step={steps[0]} variant="preview" /> + {previewStep != null && ( + <TemplateCardPreviewItem step={previewStep} variant="preview" /> + )}Also applies to: 84-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsx` around lines 33 - 35, The preview rendering in DoneScreen uses steps[0] directly which will crash if transformer(journey?.blocks ?? []) returns an empty array; update the DoneScreen component to guard access to steps by checking steps.length (or using a null/undefined fallback) before passing steps[0] into the preview renderer so you only pass a valid StepBlock or undefined/null when no steps exist, and adjust the preview component call accordingly to handle the empty case.apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx (1)
214-234:⚠️ Potential issue | 🟠 MajorAdd accessible labels to the upload icon button and file input.
Line [214] is an icon-only upload control without an accessible name, and the hidden input at Line [229] also needs one for assistive tech clarity.
Suggested fix
<IconButton component="label" + aria-label={t('Upload social image')} sx={{ @@ <StyledInput onChange={(event) => handleImageChange(event.target.files?.[0])} data-testid="SocialScreenSocialImageInput" type="file" accept="image/*" + aria-label={t('Upload social image')} />As per coding guidelines: "Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx` around lines 214 - 234, The upload control lacks accessible names: add an accessible label to the IconButton (the label wrapper) and to the hidden file input so assistive tech can identify the control. Update the IconButton (component="label") to include an aria-label (e.g., "Upload social image") and keyboard focusability if needed, and update StyledInput to include either an id and aria-labelledby pointing to that label or an aria-label attribute itself (keep data-testid and the onChange handler handleImageChange unchanged); ensure the input remains type="file" and accept="image/*" but now has an accessible name for screen readers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsx`:
- Around line 100-103: The anchor created in DoneScreen (the JSX element using
props href/component/target/startIcon) sets target="_blank" without rel,
creating reverse-tabnabbing risk; update the element to include rel="noopener
noreferrer" whenever href != null (i.e., when target is '_blank') so the
rendered anchor uses rel="noopener noreferrer" only for new-tab previews.
In `@libs/locales/en/apps-journeys-admin.json`:
- Line 1014: The localization entry currently contains a duplicated word in the
string value ("This contains contains buttons..."); update the JSON value for
that key so the string reads "This contains buttons linking to external sites.
Check them and update the links below." (locate the offending entry by searching
for the exact text "This contains contains buttons linking to external sites.
Check them and update the links below." and correct the duplicated "contains").
---
Outside diff comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx`:
- Around line 75-130: The upload flow sets setLoading(true) but only resets it
in the inner finally and always shows a success snackbar; wrap the entire
upload+mutations in a top-level try/catch/finally so setLoading(false) always
runs and move the success enqueueSnackbar into the success path (after all
awaits succeed). Specifically, in the handler around
createCloudflareUploadByFile, create a top-level try { const {data} = await
createCloudflareUploadByFile(...); if
(!data?.createCloudflareUploadByFile?.uploadUrl) throw new Error('Missing
uploadUrl'); /* perform fetch and await
journeyImageBlockUpdate/journeyImageBlockCreate/journeyImageBlockAssociationUpdate
as now */ enqueueSnackbar(t('Social image updated'),
{variant:'success',preventDuplicate:true}); } catch (error) {
enqueueSnackbar(t('Failed to update social image, please try again later'),
{variant:'error',preventDuplicate:true}); } finally { setLoading(false); } This
ensures createCloudflareUploadByFile, fetch response, and GraphQL mutations
(journeyImageBlockUpdate, journeyImageBlockCreate,
journeyImageBlockAssociationUpdate) errors are handled, loading is cleared on
every path, and success is only shown when all steps complete.
---
Duplicate comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsx`:
- Around line 33-35: The preview rendering in DoneScreen uses steps[0] directly
which will crash if transformer(journey?.blocks ?? []) returns an empty array;
update the DoneScreen component to guard access to steps by checking
steps.length (or using a null/undefined fallback) before passing steps[0] into
the preview renderer so you only pass a valid StepBlock or undefined/null when
no steps exist, and adjust the preview component call accordingly to handle the
empty case.
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx`:
- Around line 214-234: The upload control lacks accessible names: add an
accessible label to the IconButton (the label wrapper) and to the hidden file
input so assistive tech can identify the control. Update the IconButton
(component="label") to include an aria-label (e.g., "Upload social image") and
keyboard focusability if needed, and update StyledInput to include either an id
and aria-labelledby pointing to that label or an aria-label attribute itself
(keep data-testid and the onChange handler handleImageChange unchanged); ensure
the input remains type="file" and accept="image/*" but now has an accessible
name for screen readers.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsxlibs/locales/en/apps-journeys-admin.jsonlibs/locales/en/journeys-ui.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.tsx
Contributor
github-actions
Bot
temporarily deployed
to
Preview - journeys-admin
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsx (1)
102-105:⚠️ Potential issue | 🟠 MajorAdd
relwhen usingtarget="_blank"on the preview CTA.Line [104] opens a new tab, but there is no
rel="noopener noreferrer". This leaves a reverse-tabnabbing vector.🔒 Suggested fix
href={href} component={href != null ? 'a' : 'button'} target={href != null ? '_blank' : undefined} + rel={href != null ? 'noopener noreferrer' : undefined} startIcon={<Play3Icon />}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsx` around lines 102 - 105, The anchor rendered in DoneScreen (the element with props href, component, target and startIcon and using Play3Icon) opens a new tab without a rel attribute; add a rel prop when href != null/target is '_blank' (e.g., rel="noopener noreferrer") to prevent reverse-tabnabbing. Update the JSX that sets href/component/target to include rel={href != null ? 'noopener noreferrer' : undefined} (or equivalent conditional) so the link only gets rel when rendered as an external anchor.
🧹 Nitpick comments (1)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.spec.tsx (1)
389-420: Desktop test mixes desktop and mobile assertions.The test name says “desktop”, but Line [419] asserts mobile-specific copy too. This weakens responsive regression coverage and makes intent ambiguous.
🧪 Suggested cleanup
- it('renders all required components correctly for desktop', async () => { + it('renders all required components correctly', async () => { @@ - expect( - screen.getByText("A few quick edits and it's ready to share!") - ).toBeInTheDocument()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.spec.tsx` around lines 389 - 420, The desktop test 'renders all required components correctly for desktop' in LanguageScreen.spec.tsx mixes desktop and mobile assertions—remove the mobile-specific expectation that checks the string "A few quick edits and it's ready to share!" from this test; instead create a separate mobile-focused test that renders LanguageScreen in a mobile context (e.g., set the viewport or pass the mobile prop/context) and asserts that mobile copy. Update the existing desktop test to only assert desktop-only text like "Let's Get Started!" and "A few quick edits and your template will be ready to share." and ensure the new mobile test targets the mobile string to keep responsive coverage clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/locales/en/apps-journeys-admin.json`:
- Line 1037: The locale key "your.nextsteps.is" appears to be a typo and should
match the existing default-domain key "your.nextstep.is"; update the new JSON
entry to use "your.nextstep.is" (or the intended canonical key) and ensure any
consumers/reference lookups are updated to the corrected key so they resolve
consistently; search for both "your.nextsteps.is" and "your.nextstep.is" and
remove/replace the incorrect variant in the translation file and any code that
references it (e.g., translation lookup calls) to keep keys consistent.
---
Duplicate comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsx`:
- Around line 102-105: The anchor rendered in DoneScreen (the element with props
href, component, target and startIcon and using Play3Icon) opens a new tab
without a rel attribute; add a rel prop when href != null/target is '_blank'
(e.g., rel="noopener noreferrer") to prevent reverse-tabnabbing. Update the JSX
that sets href/component/target to include rel={href != null ? 'noopener
noreferrer' : undefined} (or equivalent conditional) so the link only gets rel
when rendered as an external anchor.
---
Nitpick comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.spec.tsx`:
- Around line 389-420: The desktop test 'renders all required components
correctly for desktop' in LanguageScreen.spec.tsx mixes desktop and mobile
assertions—remove the mobile-specific expectation that checks the string "A few
quick edits and it's ready to share!" from this test; instead create a separate
mobile-focused test that renders LanguageScreen in a mobile context (e.g., set
the viewport or pass the mobile prop/context) and asserts that mobile copy.
Update the existing desktop test to only assert desktop-only text like "Let's
Get Started!" and "A few quick edits and your template will be ready to share."
and ensure the new mobile test targets the mobile string to keep responsive
coverage clear.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.spec.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/DoneScreen/DoneScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.spec.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsxlibs/locales/en/apps-journeys-admin.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx
thomastayler
previously approved these changes
Feb 27, 2026
thomastayler
approved these changes
Feb 27, 2026
github-actions
Bot
temporarily deployed
to
Preview - journeys-admin
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx (1)
245-258: Remove redundant loading check in the loading branch.The inner
loading &&is unnecessary because this block is already gated byloading.Suggested cleanup
- {loading && ( - <CircularProgress - data-testid="SocialScreenSocialImageCircularProgress" - size={30} - sx={{ - position: 'absolute', - top: 0, - right: 0, - bottom: 0, - left: 0, - margin: 'auto' - }} - /> - )} + <CircularProgress + data-testid="SocialScreenSocialImageCircularProgress" + size={30} + sx={{ + position: 'absolute', + top: 0, + right: 0, + bottom: 0, + left: 0, + margin: 'auto' + }} + />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx` around lines 245 - 258, The JSX currently nests a redundant conditional (`loading &&`) inside a parent block already guarded by `loading`; in the SocialScreenSocialImage component remove the inner `loading &&` check so the CircularProgress (the element with data-testid "SocialScreenSocialImageCircularProgress") is rendered directly inside the outer loading branch; ensure you only leave the single outer conditional that renders CircularProgress to avoid the duplicate guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.tsx`:
- Line 210: The current JSX expression `{`'${journey?.title ?? ''}'`}` renders
literal empty quotes when journey?.title is falsy; update the LanguageScreen JSX
to conditionally render the quoted title only when journey?.title is non-empty
(e.g., check `journey?.title` before rendering) so that the UI does not show
`''` — locate the template that uses `journey?.title` (the expression currently
producing `'{journey?.title ?? ''}'`) and wrap it in a conditional (or render an
alternative) to avoid outputting empty quoted text.
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx`:
- Line 212: The hardcoded aria-label "Edit social image" in the
SocialScreenSocialImage component must be localized; replace the literal string
with a value obtained from your app's i18n helper (e.g., using
useTranslation().t or intl.formatMessage) and pass that localized string to the
edit button's aria-label prop (reference: SocialScreenSocialImage component and
the edit button element with aria-label). Add a corresponding translation key
(e.g., "socialScreen.editSocialImage" or "editSocialImage") to your locale files
and use that key when calling the translation function so the aria-label is
translated for all languages.
---
Nitpick comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsx`:
- Around line 245-258: The JSX currently nests a redundant conditional (`loading
&&`) inside a parent block already guarded by `loading`; in the
SocialScreenSocialImage component remove the inner `loading &&` check so the
CircularProgress (the element with data-testid
"SocialScreenSocialImageCircularProgress") is rendered directly inside the outer
loading branch; ensure you only leave the single outer conditional that renders
CircularProgress to avoid the duplicate guard.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.spec.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreen.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreenSocialImage/SocialScreenSocialImage.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/TextScreen/TextScreen.tsxlibs/locales/en/apps-journeys-admin.json
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/TextScreen/TextScreen.tsx
- apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/SocialScreen/SocialScreen.tsx
- apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LanguageScreen/LanguageScreen.spec.tsx
- apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksScreen.tsx
csiyang
deleted the
siyangcao/nes-1375-update-minor-design-details-for-screens
branch
This was referenced Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests