Skip to content

Commit

Permalink
Ensure only variant path part is changed
Browse files Browse the repository at this point in the history
  • Loading branch information
amoore108 committed Jan 22, 2025
1 parent 15846d8 commit 7534a7f
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 12 deletions.
30 changes: 23 additions & 7 deletions src/app/components/Header/ScriptLink/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { render } from '../../react-testing-library-with-providers';
import ScriptLinkContainer from '.';
import ScriptLink from '.';

const enabledToggleState = {
scriptLink: {
Expand All @@ -17,7 +17,7 @@ describe(`Script Link`, () => {
});

it('should render correctly', () => {
const { container } = render(<ScriptLinkContainer />, {
const { container } = render(<ScriptLink />, {
toggles: enabledToggleState,
service: 'serbian',
variant: 'lat',
Expand All @@ -27,7 +27,7 @@ describe(`Script Link`, () => {
});

describe('assertions', () => {
describe.each(['canonical', 'amp'])('%s', platform => {
describe.each(['canonical', 'amp', 'lite'])('%s', platform => {
it.each`
service | variant | pageType | path | variantPath
${'serbian'} | ${'lat'} | ${'article'} | ${'/serbian/articles/c805k05kr73o/lat'} | ${'/serbian/articles/c805k05kr73o/cyr'}
Expand All @@ -46,12 +46,18 @@ describe(`Script Link`, () => {
'Script Link should contain link to other variant when on $service $variant $pageType',
({ service, variant, path, variantPath }) => {
const isAmp = platform === 'amp';
const isLite = platform === 'lite';

const { container } = render(<ScriptLinkContainer />, {
let pathToUse = path;

if (isAmp) pathToUse = `${path}.amp`;
if (isLite) pathToUse = `${path}.lite`;

const { container } = render(<ScriptLink />, {
toggles: enabledToggleState,
service,
variant,
pathname: isAmp ? `${path}.amp` : path,
pathname: pathToUse,
isAmp,
});

Expand All @@ -73,20 +79,30 @@ describe(`Script Link`, () => {
},
};

const { container } = render(<ScriptLinkContainer />, {
const { container } = render(<ScriptLink />, {
toggles: disabledToggleState,
});

expect(container).toBeEmptyDOMElement();
});

it('should not render when an alternate variant is not defined', () => {
const { container } = render(<ScriptLinkContainer />, {
const { container } = render(<ScriptLink />, {
toggles: enabledToggleState,
service: 'pidgin',
pathname: '/pidgin',
});

expect(container).toBeEmptyDOMElement();
});

it('should not render if pathname is not defined', () => {
const { container } = render(<ScriptLink />, {
toggles: enabledToggleState,
service: 'serbian',
pathname: '',
});

expect(container).toBeEmptyDOMElement();
});
});
18 changes: 13 additions & 5 deletions src/app/components/Header/ScriptLink/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,25 @@ const ScriptLink = () => {

const { text, variant: alternateVariant } = scriptLink || {};

if (!pathname) return null;
if (!scriptLinkEnabled) return null;
if (!alternateVariant) return null;

const pathPartsWithoutExtension = pathname
.replace(/\.[^/.]+$/, '') // remove any extensions
.split('/');

const variantIndex = pathPartsWithoutExtension.indexOf(
currentVariant as string,
);

pathPartsWithoutExtension[variantIndex] = alternateVariant;
const newPath = pathPartsWithoutExtension.join('/').replace('.amp', ''); // we don't want to link to AMP pages directly;

return (
<a
css={styles.link}
href={
pathname
.replace(currentVariant as string, alternateVariant)
.replace('.amp', '') // we don't want to link to AMP pages directly
}
href={newPath}
data-variant={alternateVariant}
className="focusIndicatorRemove"
>
Expand Down

0 comments on commit 7534a7f

Please sign in to comment.