Skip to content
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

Jump to bottom

Election banner height fixes #12127

Merged
merged 9 commits into from
Nov 4, 2024
Merged

Election banner height fixes #12127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ce0f596
Election banner height fixes
amoore108 Nov 2, 2024
05b804d
Account for live updates label
amoore108 Nov 2, 2024
ae89291
more height adjustments
amoore108 Nov 3, 2024
0e4a27c
Default to `english` banner for News service
amoore108 Nov 3, 2024
b554517
Test fixes
amoore108 Nov 3, 2024
4c32688
Update index.test.tsx
amoore108 Nov 3, 2024
2247ea8
Remove hiding at <=320px
amoore108 Nov 3, 2024
d7c169e
Fix padding on homepage banner
amoore108 Nov 3, 2024
1a1bf51
Merge branch 'latest' into election-banner-height-fixes
amoore108 Nov 4, 2024
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
10 changes: 6 additions & 4 deletions src/app/components/Embeds/EmbedHtml/index.styles.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import pixelsToRem from '#app/utilities/pixelsToRem';
import { css, Theme } from '@emotion/react';

const styles = {
Expand All @@ -21,14 +20,17 @@ const styles = {
// TODO: Remove this styling after the US Elections
electionBannerOverrides: ({ mq }: Theme) =>
css({
[`@media (max-width:${pixelsToRem(320)}rem)`]: {
display: 'none',
},
paddingLeft: 0,
paddingRight: 0,

[mq.GROUP_2_MIN_WIDTH]: {
paddingLeft: 0,
paddingRight: 0,
},
[mq.GROUP_4_MIN_WIDTH]: {
paddingLeft: 0,
paddingRight: 0,
},
}),
};

Expand Down
6 changes: 3 additions & 3 deletions src/app/pages/ArticlePage/ElectionBanner/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export default {
heights: {
desktop: 465,
tablet: 480,
mobile: 540,
desktop: 350,
tablet: 320,
mobile: 315,
},
iframeSrc:
'include/vjafwest/1365-2024-us-presidential-election-banner/{service}/app',
Expand Down
12 changes: 2 additions & 10 deletions src/app/pages/ArticlePage/ElectionBanner/index.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ export default {
electionBannerWrapper: ({ spacings }: Theme) =>
css({
marginBottom: `${spacings.FULL}rem`,

[`@media (max-width:${pixelsToRem(320)}rem)`]: {
display: 'none',
},
}),

electionBannerIframe: ({ mq }: Theme) =>
Expand All @@ -18,7 +14,7 @@ export default {
width: '100%',
height: `${pixelsToRem(BANNER_CONFIG.heights.mobile)}rem`,

[mq.GROUP_2_MIN_WIDTH]: {
[mq.GROUP_3_MIN_WIDTH]: {
height: `${pixelsToRem(BANNER_CONFIG.heights.tablet)}rem`,
},
[mq.GROUP_4_MIN_WIDTH]: {
Expand All @@ -31,10 +27,6 @@ export default {
overflow: 'hidden',
marginBottom: `${spacings.FULL}rem`,

[`@media (max-width:${pixelsToRem(320)}rem)`]: {
display: 'none',
},

'> div': { padding: '0' },
'& amp-img': {
maxWidth: 640,
Expand All @@ -45,7 +37,7 @@ export default {
width: '100%',
height: `${pixelsToRem(BANNER_CONFIG.heights.mobile)}rem`,

[mq.GROUP_2_MIN_WIDTH]: {
[mq.GROUP_3_MIN_WIDTH]: {
height: `${pixelsToRem(BANNER_CONFIG.heights.tablet)}rem`,
},
[mq.GROUP_4_MIN_WIDTH]: {
Expand Down
24 changes: 22 additions & 2 deletions src/app/pages/ArticlePage/ElectionBanner/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe('ElectionBanner', () => {

const configSrc = BANNER_CONFIG[
appEnv === 'live' ? 'iframeSrc' : 'iframeDevSrc'
].replace('{service}', 'news');
].replace('{service}', 'english');

const domain = isAmp
? process.env.SIMORGH_INCLUDES_BASE_AMP_URL
Expand Down Expand Up @@ -123,15 +123,35 @@ describe('ElectionBanner', () => {
const iframe = wrappingEl.querySelector('iframe, amp-iframe');
const iframeSrc = iframe?.getAttribute('src');

expect(iframeSrc).not.toContain('turkce');
expect(iframeSrc).not.toContain('/turkce/');
expect(iframeSrc).toContain('turkish');
});

it('should render the correct VJ URL for News service', () => {
const { getByTestId } = render(
<ElectionBanner aboutTags={mockAboutTags} taggings={mockTaggings} />,
{
toggles: { electionBanner: { enabled: true } },
isAmp,
service: 'news',
},
);

const wrappingEl = getByTestId(ELEMENT_ID);

const iframe = wrappingEl.querySelector('iframe, amp-iframe');
const iframeSrc = iframe?.getAttribute('src');

expect(iframeSrc).not.toContain('/news/');
expect(iframeSrc).toContain('english');
});

it('should not render ElectionBanner when taggings contain the editorialSensitivityId', () => {
const { queryByTestId } = render(
<ElectionBanner
aboutTags={mockAboutTags}
taggings={[
...mockTaggings,
{
predicate:
'http://www.bbc.co.uk/ontologies/bbc/editorialSensitivity',
Expand Down
4 changes: 3 additions & 1 deletion src/app/pages/ArticlePage/ElectionBanner/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const handleUrlServiceTransform = (url: string, service: Services) => {
switch (service) {
case 'turkce':
return url.replace('{service}', 'turkish');
case 'news':
return url.replace('{service}', 'english');
default:
return url.replace('{service}', service);
}
Expand All @@ -43,7 +45,7 @@ export default function ElectionBanner({ aboutTags, taggings }: Props) {
usElectionThingId,
} = BANNER_CONFIG;

const isEditoriallySensitive = taggings?.find(({ value }) =>
const isEditoriallySensitive = taggings?.some(({ value }) =>
value.includes(editorialSensitivityId),
);

Expand Down
Loading