From 1236ea27e0a5e1d16461f2e941e8761428b3c169 Mon Sep 17 00:00:00 2001 From: Marjan Kalanaki Date: Wed, 11 Dec 2024 17:41:42 +0000 Subject: [PATCH 1/6] Optimize useIsAndroid hook Co-authored-by: Jamie B <53781962+JamieB-gu@users.noreply.github.com> --- .../src/components/Carousel.importable.tsx | 6 ++-- .../components/OnwardsUpper.importable.tsx | 8 +++-- .../StickyBottomBanner.importable.tsx | 3 +- dotcom-rendering/src/lib/useIsAndroid.ts | 24 ------------- .../src/lib/useIsBridgetCompatible.ts | 23 +++++++----- .../lib/useIsHorizontalScrollingSupported.ts | 35 +++++++++++++++++++ 6 files changed, 58 insertions(+), 41 deletions(-) delete mode 100644 dotcom-rendering/src/lib/useIsAndroid.ts create mode 100644 dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts diff --git a/dotcom-rendering/src/components/Carousel.importable.tsx b/dotcom-rendering/src/components/Carousel.importable.tsx index cd88df7e693..520558de9be 100644 --- a/dotcom-rendering/src/components/Carousel.importable.tsx +++ b/dotcom-rendering/src/components/Carousel.importable.tsx @@ -12,7 +12,7 @@ import { ArticleDesign, type ArticleFormat } from '../lib/articleFormat'; import { formatAttrString } from '../lib/formatAttrString'; import { getSourceImageUrl } from '../lib/getSourceImageUrl_temp_fix'; import { getZIndex } from '../lib/getZIndex'; -import { useIsAndroid } from '../lib/useIsAndroid'; +import { useIsHorizontalScrollingSupported } from '../lib/useIsHorizontalScrollingSupported'; import { palette as themePalette } from '../palette'; import type { Branding } from '../types/branding'; import type { StarRating } from '../types/content'; @@ -798,7 +798,7 @@ export const Carousel = ({ const [index, setIndex] = useState(0); const [maxIndex, setMaxIndex] = useState(0); - const isAndroid = useIsAndroid(); + const isHorizontalScrollingSupported = useIsHorizontalScrollingSupported(); const arrowName = 'carousel-small-arrow'; @@ -907,7 +907,7 @@ export const Carousel = ({ // when index changes and compare it against the prior maxIndex only. useEffect(() => setMaxIndex((m) => Math.max(index, m)), [index]); - if (isAndroid) { + if (!isHorizontalScrollingSupported) { return null; } diff --git a/dotcom-rendering/src/components/OnwardsUpper.importable.tsx b/dotcom-rendering/src/components/OnwardsUpper.importable.tsx index bec9540d28c..f8011ef8d86 100644 --- a/dotcom-rendering/src/components/OnwardsUpper.importable.tsx +++ b/dotcom-rendering/src/components/OnwardsUpper.importable.tsx @@ -6,7 +6,7 @@ import { Pillar, } from '../lib/articleFormat'; import type { EditionId } from '../lib/edition'; -import { useIsAndroid } from '../lib/useIsAndroid'; +import { useIsHorizontalScrollingSupported } from '../lib/useIsHorizontalScrollingSupported'; import { palette } from '../palette'; import type { OnwardsSource } from '../types/onwards'; import type { TagType } from '../types/tag'; @@ -214,8 +214,10 @@ export const OnwardsUpper = ({ discussionApiUrl, absoluteServerTimes, }: Props) => { - const isAndroid = useIsAndroid(); - if (isAndroid) return null; + + const isHorizontalScrollingSupported = useIsHorizontalScrollingSupported(); + + if (!isHorizontalScrollingSupported) return null; // Related content can be a collection of articles based on // two things, 1: A popular tag, or 2: A generic text match diff --git a/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx b/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx index 7cc2a464f58..e7610261441 100644 --- a/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx +++ b/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx @@ -16,7 +16,6 @@ import { pickMessage } from '../lib/messagePicker'; import { useIsSignedIn } from '../lib/useAuthStatus'; import { useBraze } from '../lib/useBraze'; import { useCountryCode } from '../lib/useCountryCode'; -import { useIsAndroid } from '../lib/useIsAndroid'; import { useSignInGateWillShow } from '../lib/useSignInGateWillShow'; import type { TagType } from '../types/tag'; import { useConfig } from './ConfigContext'; @@ -246,7 +245,7 @@ export const StickyBottomBanner = ({ isPreview, currentLocaleCode: countryCode, }); - const isAndroidWebview = !!useIsAndroid(); + const isAndroidWebview = false; // TODO: fix in upcoming commit !!useIsAndroid(); useEffect(() => { setAsyncArticleCounts(getArticleCounts(pageId, tags, contentType)); diff --git a/dotcom-rendering/src/lib/useIsAndroid.ts b/dotcom-rendering/src/lib/useIsAndroid.ts deleted file mode 100644 index 6b1541f64b4..00000000000 --- a/dotcom-rendering/src/lib/useIsAndroid.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { useEffect, useState } from 'react'; -import { useConfig } from '../components/ConfigContext'; - -/** - * @deprecated this is a temporary solution to handle the fact - * that horizontal scrolling is broken in the android app for - * web views - */ -export const useIsAndroid = (): boolean | undefined => { - const { renderingTarget } = useConfig(); - - const [isAndroid, setIsAndroid] = useState( - renderingTarget === 'Web' ? false : undefined, - ); - - useEffect(() => { - if (renderingTarget === 'Web') { - return setIsAndroid(false); - } - setIsAndroid(() => /android/i.test(window.navigator.userAgent)); - }, [renderingTarget]); - - return isAndroid; -}; diff --git a/dotcom-rendering/src/lib/useIsBridgetCompatible.ts b/dotcom-rendering/src/lib/useIsBridgetCompatible.ts index 6a60b8f7157..d44314defc1 100644 --- a/dotcom-rendering/src/lib/useIsBridgetCompatible.ts +++ b/dotcom-rendering/src/lib/useIsBridgetCompatible.ts @@ -10,15 +10,20 @@ export const useIsBridgetCompatible = ( ); useEffect(() => { - void getEnvironmentClient() - .nativeThriftPackageVersion() - .then((bridgetVersion) => { - setIsCompatible(compare(bridgetVersion, requiredVersion, '>=')); - }) - .catch((error) => { - setIsCompatible(false); - console.log('nativeThriftPackageVersion', { error }); - }); + hasMinimumBridgetVersion(requiredVersion).then(setIsCompatible); }, [requiredVersion]); return isCompatible; }; + +export const hasMinimumBridgetVersion = ( + requiredVersion: string, +): Promise => + getEnvironmentClient() + .nativeThriftPackageVersion() + .then((bridgetVersion) => { + return compare(bridgetVersion, requiredVersion, '>='); + }) + .catch((error) => { + console.log('nativeThriftPackageVersion', { error }); + return false; + }); diff --git a/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts b/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts new file mode 100644 index 00000000000..fc093a5f5e0 --- /dev/null +++ b/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts @@ -0,0 +1,35 @@ +import { useEffect, useState } from 'react'; +import { useConfig } from '../components/ConfigContext'; +import { hasMinimumBridgetVersion } from './useIsBridgetCompatible'; + +/** + * this is a solution to handle the fact that horizontal + * scrolling is broken in older android app versions + * for web views + */ +export const useIsHorizontalScrollingSupported = (): boolean => { + const { renderingTarget } = useConfig(); + + const [isSupported, setIsSupported] = useState(true); + + const setHorizontalScrollingSupported = ( + hasDisableArticleSwipeApi: boolean, + ) => { + // If we don't have article swipe api available in bridget + // then horizontal scrolling is not supported on android + if (!hasDisableArticleSwipeApi) { + const isAndroid = /android/i.test(window.navigator.userAgent); + setIsSupported(!isAndroid); + } + }; + + useEffect(() => { + if (renderingTarget === 'Apps') { + hasMinimumBridgetVersion('8.1.1').then( + setHorizontalScrollingSupported, + ); + } + }, [renderingTarget]); + + return isSupported; +}; From fa820cd57c6d1c4f616bbffdbc3a33b663f9001a Mon Sep 17 00:00:00 2001 From: Marjan Kalanaki Date: Fri, 13 Dec 2024 12:48:06 +0000 Subject: [PATCH 2/6] add touch events for carousel for apps rendering target --- .../src/components/Carousel.importable.tsx | 16 ++++++++++++++++ .../src/components/Carousel.stories.tsx | 7 +++++++ .../components/FetchOnwardsData.importable.tsx | 4 ++++ dotcom-rendering/src/components/Island.test.tsx | 1 + .../src/components/OnwardsUpper.importable.tsx | 5 +++++ dotcom-rendering/src/layouts/AudioLayout.tsx | 7 ++++++- dotcom-rendering/src/layouts/CommentLayout.tsx | 2 ++ dotcom-rendering/src/layouts/DecideLayout.tsx | 2 ++ dotcom-rendering/src/layouts/FrontLayout.tsx | 4 ++++ dotcom-rendering/src/layouts/ImmersiveLayout.tsx | 2 ++ .../src/layouts/InteractiveLayout.tsx | 2 ++ dotcom-rendering/src/layouts/LiveLayout.tsx | 2 ++ .../src/layouts/NewsletterSignupLayout.tsx | 11 ++++++++++- dotcom-rendering/src/layouts/PictureLayout.tsx | 2 ++ dotcom-rendering/src/layouts/ShowcaseLayout.tsx | 2 ++ dotcom-rendering/src/layouts/StandardLayout.tsx | 2 ++ .../src/lib/useIsBridgetCompatible.ts | 6 +++++- .../src/lib/useIsHorizontalScrollingSupported.ts | 3 ++- 18 files changed, 76 insertions(+), 4 deletions(-) diff --git a/dotcom-rendering/src/components/Carousel.importable.tsx b/dotcom-rendering/src/components/Carousel.importable.tsx index 520558de9be..3fac3f6d446 100644 --- a/dotcom-rendering/src/components/Carousel.importable.tsx +++ b/dotcom-rendering/src/components/Carousel.importable.tsx @@ -32,6 +32,8 @@ import { ContainerOverrides } from './ContainerOverrides'; import { FormatBoundary } from './FormatBoundary'; import { Hide } from './Hide'; import { LeftColumn } from './LeftColumn'; +import { getInteractionClient } from '../lib/bridgetApi'; +import { RenderingTarget } from 'src/types/renderingTarget'; type Props = { heading: string; @@ -42,6 +44,7 @@ type Props = { leftColSize: LeftColSize; discussionApiUrl: string; absoluteServerTimes: boolean; + renderingTarget: RenderingTarget; }; type ArticleProps = Props & { @@ -792,6 +795,7 @@ export const Carousel = ({ discussionApiUrl, isOnwardContent = true, absoluteServerTimes, + renderingTarget, ...props }: ArticleProps | FrontProps) => { const carouselRef = useRef(null); @@ -800,6 +804,8 @@ export const Carousel = ({ const [maxIndex, setMaxIndex] = useState(0); const isHorizontalScrollingSupported = useIsHorizontalScrollingSupported(); + const isApps = renderingTarget === 'Apps'; + const arrowName = 'carousel-small-arrow'; const isCuratedContent = onwardsSource === 'curated-content'; @@ -907,6 +913,14 @@ export const Carousel = ({ // when index changes and compare it against the prior maxIndex only. useEffect(() => setMaxIndex((m) => Math.max(index, m)), [index]); + const onTouchStart = async () => { + await getInteractionClient().disableArticleSwipe(true); + }; + + const onTouchEnd = async () => { + await getInteractionClient().disableArticleSwipe(false); + }; + if (!isHorizontalScrollingSupported) { return null; } @@ -974,6 +988,8 @@ export const Carousel = ({ ref={carouselRef} data-component={`carousel-small | maxIndex-${maxIndex}`} data-heatphan-type="carousel" + onTouchStart={isApps ? onTouchStart : undefined} + onTouchEnd={isApps ? onTouchEnd : undefined} > {trails.map((trail, i) => { const { diff --git a/dotcom-rendering/src/components/Carousel.stories.tsx b/dotcom-rendering/src/components/Carousel.stories.tsx index bca2c07f957..1cf1702cf2d 100644 --- a/dotcom-rendering/src/components/Carousel.stories.tsx +++ b/dotcom-rendering/src/components/Carousel.stories.tsx @@ -270,6 +270,7 @@ export const Headlines: StoryObj = ({ format }: StoryProps) => { leftColSize="compact" discussionApiUrl={discussionApiUrl} absoluteServerTimes={true} + renderingTarget="Web" /> ); @@ -295,6 +296,7 @@ export const SingleItemCarousel = () => { leftColSize="compact" discussionApiUrl={discussionApiUrl} absoluteServerTimes={true} + renderingTarget="Web" /> ); @@ -349,6 +351,7 @@ export const SingleOpinionCarousel = () => { leftColSize="compact" discussionApiUrl={discussionApiUrl} absoluteServerTimes={true} + renderingTarget="Web" /> ); @@ -379,6 +382,7 @@ export const Immersive = () => { leftColSize="compact" discussionApiUrl={discussionApiUrl} absoluteServerTimes={true} + renderingTarget="Web" /> @@ -424,6 +428,7 @@ export const SpecialReportAlt = () => { leftColSize="compact" discussionApiUrl={discussionApiUrl} absoluteServerTimes={true} + renderingTarget="Web" /> ); @@ -639,6 +644,7 @@ export const AllCards = () => { discussionApiUrl={discussionApiUrl} format={defaultFormat} absoluteServerTimes={true} + renderingTarget="Web" /> ); @@ -663,6 +669,7 @@ export const FrontCarousel = () => ( discussionApiUrl={discussionApiUrl} palette="PodcastPalette" absoluteServerTimes={true} + renderingTarget="Web" /> diff --git a/dotcom-rendering/src/components/FetchOnwardsData.importable.tsx b/dotcom-rendering/src/components/FetchOnwardsData.importable.tsx index 3cd570f47f2..2f3d7f9823a 100644 --- a/dotcom-rendering/src/components/FetchOnwardsData.importable.tsx +++ b/dotcom-rendering/src/components/FetchOnwardsData.importable.tsx @@ -9,6 +9,7 @@ import type { OnwardsSource } from '../types/onwards'; import type { FETrailType, TrailType } from '../types/trails'; import { Carousel } from './Carousel.importable'; import { Placeholder } from './Placeholder'; +import { RenderingTarget } from 'src/types/renderingTarget'; type Props = { url: string; @@ -17,6 +18,7 @@ type Props = { format: ArticleFormat; discussionApiUrl: string; absoluteServerTimes: boolean; + renderingTarget: RenderingTarget; }; type OnwardsResponse = { @@ -37,6 +39,7 @@ export const FetchOnwardsData = ({ format, discussionApiUrl, absoluteServerTimes, + renderingTarget, }: Props) => { const { data, error } = useApi(url); @@ -85,6 +88,7 @@ export const FetchOnwardsData = ({ } discussionApiUrl={discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> ); diff --git a/dotcom-rendering/src/components/Island.test.tsx b/dotcom-rendering/src/components/Island.test.tsx index 00aebbcd40a..bc0fcffeb56 100644 --- a/dotcom-rendering/src/components/Island.test.tsx +++ b/dotcom-rendering/src/components/Island.test.tsx @@ -205,6 +205,7 @@ describe('Island: server-side rendering', () => { shortUrlId="" discussionApiUrl="" absoluteServerTimes={true} + renderingTarget="Web" /> , ), diff --git a/dotcom-rendering/src/components/OnwardsUpper.importable.tsx b/dotcom-rendering/src/components/OnwardsUpper.importable.tsx index f8011ef8d86..a1e115ea827 100644 --- a/dotcom-rendering/src/components/OnwardsUpper.importable.tsx +++ b/dotcom-rendering/src/components/OnwardsUpper.importable.tsx @@ -12,6 +12,7 @@ import type { OnwardsSource } from '../types/onwards'; import type { TagType } from '../types/tag'; import { FetchOnwardsData } from './FetchOnwardsData.importable'; import { Section } from './Section'; +import { RenderingTarget } from 'src/types/renderingTarget'; type PillarForContainer = | 'headlines' @@ -179,6 +180,7 @@ type Props = { shortUrlId: string; discussionApiUrl: string; absoluteServerTimes: boolean; + renderingTarget: RenderingTarget; }; /** @@ -213,6 +215,7 @@ export const OnwardsUpper = ({ shortUrlId, discussionApiUrl, absoluteServerTimes, + renderingTarget, }: Props) => { const isHorizontalScrollingSupported = useIsHorizontalScrollingSupported(); @@ -311,6 +314,7 @@ export const OnwardsUpper = ({ format={format} discussionApiUrl={discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> )} @@ -326,6 +330,7 @@ export const OnwardsUpper = ({ format={format} discussionApiUrl={discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> )} diff --git a/dotcom-rendering/src/layouts/AudioLayout.tsx b/dotcom-rendering/src/layouts/AudioLayout.tsx index ba9a7dd2a52..76b3c04400e 100644 --- a/dotcom-rendering/src/layouts/AudioLayout.tsx +++ b/dotcom-rendering/src/layouts/AudioLayout.tsx @@ -44,6 +44,7 @@ import type { NavType } from '../model/extract-nav'; import { palette as themePalette } from '../palette'; import type { ArticleDeprecated } from '../types/article'; import { BannerWrapper, Stuck } from './lib/stickiness'; +import { RenderingTarget } from '../types/renderingTarget'; const AudioGrid = ({ children }: { children: React.ReactNode }) => (
{ - const { article, format } = props; + const { article, format, renderingTarget } = props; const audioData = getAudioData(article.mainMediaElements); const { @@ -495,6 +498,7 @@ export const AudioLayout = (props: WebProps) => { article.config.discussionApiUrl } absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> @@ -518,6 +522,7 @@ export const AudioLayout = (props: WebProps) => { shortUrlId={article.config.shortUrlId} discussionApiUrl={article.config.discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> {showComments && ( diff --git a/dotcom-rendering/src/layouts/CommentLayout.tsx b/dotcom-rendering/src/layouts/CommentLayout.tsx index 3e519da0bee..0cfb553de80 100644 --- a/dotcom-rendering/src/layouts/CommentLayout.tsx +++ b/dotcom-rendering/src/layouts/CommentLayout.tsx @@ -737,6 +737,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => { article.config.discussionApiUrl } absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> @@ -760,6 +761,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => { shortUrlId={article.config.shortUrlId} discussionApiUrl={article.config.discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> diff --git a/dotcom-rendering/src/layouts/DecideLayout.tsx b/dotcom-rendering/src/layouts/DecideLayout.tsx index b55f4973791..6ec7b68a102 100644 --- a/dotcom-rendering/src/layouts/DecideLayout.tsx +++ b/dotcom-rendering/src/layouts/DecideLayout.tsx @@ -263,6 +263,7 @@ const DecideLayoutWeb = ({ article={article} NAV={NAV} format={format} + renderingTarget={renderingTarget} /> ); case ArticleDesign.Audio: @@ -271,6 +272,7 @@ const DecideLayoutWeb = ({ article={article} format={format} NAV={NAV} + renderingTarget={renderingTarget} /> ); default: diff --git a/dotcom-rendering/src/layouts/FrontLayout.tsx b/dotcom-rendering/src/layouts/FrontLayout.tsx index f393a550126..5ecb1d59358 100644 --- a/dotcom-rendering/src/layouts/FrontLayout.tsx +++ b/dotcom-rendering/src/layouts/FrontLayout.tsx @@ -51,6 +51,7 @@ import type { } from '../types/front'; import { pageSkinContainer } from './lib/pageSkin'; import { BannerWrapper, Stuck } from './lib/stickiness'; +import { useConfig } from '../components/ConfigContext'; interface Props { front: DCRFrontType; @@ -114,6 +115,8 @@ export const FrontLayout = ({ front, NAV }: Props) => { editionId, } = front; + const { renderingTarget } = useConfig(); + const renderAds = canRenderAds(front); const hasPageSkin = renderAds && hasPageSkinConfig; @@ -618,6 +621,7 @@ export const FrontLayout = ({ front, NAV }: Props) => { absoluteServerTimes={ absoluteServerTimes } + renderingTarget={renderingTarget} /> diff --git a/dotcom-rendering/src/layouts/ImmersiveLayout.tsx b/dotcom-rendering/src/layouts/ImmersiveLayout.tsx index ef6fe66afdd..eb99a2fa83c 100644 --- a/dotcom-rendering/src/layouts/ImmersiveLayout.tsx +++ b/dotcom-rendering/src/layouts/ImmersiveLayout.tsx @@ -806,6 +806,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => { article.config.discussionApiUrl } absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> @@ -829,6 +830,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => { shortUrlId={article.config.shortUrlId} discussionApiUrl={article.config.discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> diff --git a/dotcom-rendering/src/layouts/InteractiveLayout.tsx b/dotcom-rendering/src/layouts/InteractiveLayout.tsx index d58a3031916..c7e5e65ca54 100644 --- a/dotcom-rendering/src/layouts/InteractiveLayout.tsx +++ b/dotcom-rendering/src/layouts/InteractiveLayout.tsx @@ -647,6 +647,7 @@ export const InteractiveLayout = (props: WebProps | AppsProps) => { article.config.discussionApiUrl } absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> @@ -670,6 +671,7 @@ export const InteractiveLayout = (props: WebProps | AppsProps) => { shortUrlId={article.config.shortUrlId} discussionApiUrl={article.config.discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> diff --git a/dotcom-rendering/src/layouts/LiveLayout.tsx b/dotcom-rendering/src/layouts/LiveLayout.tsx index 58f9e6cb261..9ac0c8ee771 100644 --- a/dotcom-rendering/src/layouts/LiveLayout.tsx +++ b/dotcom-rendering/src/layouts/LiveLayout.tsx @@ -973,6 +973,7 @@ export const LiveLayout = (props: WebProps | AppsProps) => { article.config.discussionApiUrl } absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> @@ -998,6 +999,7 @@ export const LiveLayout = (props: WebProps | AppsProps) => { shortUrlId={article.config.shortUrlId} discussionApiUrl={article.config.discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> diff --git a/dotcom-rendering/src/layouts/NewsletterSignupLayout.tsx b/dotcom-rendering/src/layouts/NewsletterSignupLayout.tsx index 13fd98b8682..6b10fd4638c 100644 --- a/dotcom-rendering/src/layouts/NewsletterSignupLayout.tsx +++ b/dotcom-rendering/src/layouts/NewsletterSignupLayout.tsx @@ -41,11 +41,13 @@ import { isValidUrl } from '../lib/isValidUrl'; import type { NavType } from '../model/extract-nav'; import type { ArticleDeprecated } from '../types/article'; import { BannerWrapper, Stuck } from './lib/stickiness'; +import { RenderingTarget } from '../types/renderingTarget'; type Props = { article: ArticleDeprecated; NAV: NavType; format: ArticleFormat; + renderingTarget: RenderingTarget; }; const mainColWrapperStyle = css` @@ -183,7 +185,12 @@ const getMainMediaCaptions = ( : undefined, ); -export const NewsletterSignupLayout = ({ article, NAV, format }: Props) => { +export const NewsletterSignupLayout = ({ + article, + NAV, + format, + renderingTarget, +}: Props) => { const { promotedNewsletter, config: { host, hasSurveyAd }, @@ -435,6 +442,7 @@ export const NewsletterSignupLayout = ({ article, NAV, format }: Props) => { article.config.discussionApiUrl } absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> @@ -458,6 +466,7 @@ export const NewsletterSignupLayout = ({ article, NAV, format }: Props) => { shortUrlId={article.config.shortUrlId} discussionApiUrl={article.config.discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> diff --git a/dotcom-rendering/src/layouts/PictureLayout.tsx b/dotcom-rendering/src/layouts/PictureLayout.tsx index 2cde2c9bf0d..2b452159a36 100644 --- a/dotcom-rendering/src/layouts/PictureLayout.tsx +++ b/dotcom-rendering/src/layouts/PictureLayout.tsx @@ -593,6 +593,7 @@ export const PictureLayout = (props: WebProps | AppsProps) => { article.config.discussionApiUrl } absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> @@ -619,6 +620,7 @@ export const PictureLayout = (props: WebProps | AppsProps) => { shortUrlId={article.config.shortUrlId} discussionApiUrl={article.config.discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> )} diff --git a/dotcom-rendering/src/layouts/ShowcaseLayout.tsx b/dotcom-rendering/src/layouts/ShowcaseLayout.tsx index 5d2f67caf8b..58da9e93f01 100644 --- a/dotcom-rendering/src/layouts/ShowcaseLayout.tsx +++ b/dotcom-rendering/src/layouts/ShowcaseLayout.tsx @@ -705,6 +705,7 @@ export const ShowcaseLayout = (props: WebProps | AppsProps) => { article.config.discussionApiUrl } absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> @@ -728,6 +729,7 @@ export const ShowcaseLayout = (props: WebProps | AppsProps) => { shortUrlId={article.config.shortUrlId} discussionApiUrl={article.config.discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> diff --git a/dotcom-rendering/src/layouts/StandardLayout.tsx b/dotcom-rendering/src/layouts/StandardLayout.tsx index e15db2a2e2f..651a009e1e7 100644 --- a/dotcom-rendering/src/layouts/StandardLayout.tsx +++ b/dotcom-rendering/src/layouts/StandardLayout.tsx @@ -884,6 +884,7 @@ export const StandardLayout = (props: WebProps | AppProps) => { article.config.discussionApiUrl } absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> @@ -907,6 +908,7 @@ export const StandardLayout = (props: WebProps | AppProps) => { shortUrlId={article.config.shortUrlId} discussionApiUrl={article.config.discussionApiUrl} absoluteServerTimes={absoluteServerTimes} + renderingTarget={renderingTarget} /> {showComments && ( diff --git a/dotcom-rendering/src/lib/useIsBridgetCompatible.ts b/dotcom-rendering/src/lib/useIsBridgetCompatible.ts index d44314defc1..ee9e338ac20 100644 --- a/dotcom-rendering/src/lib/useIsBridgetCompatible.ts +++ b/dotcom-rendering/src/lib/useIsBridgetCompatible.ts @@ -21,7 +21,11 @@ export const hasMinimumBridgetVersion = ( getEnvironmentClient() .nativeThriftPackageVersion() .then((bridgetVersion) => { - return compare(bridgetVersion, requiredVersion, '>='); + return compare( + bridgetVersion.replace(/^v/i, ''), + requiredVersion.replace(/^v/i, ''), + '>=', + ); }) .catch((error) => { console.log('nativeThriftPackageVersion', { error }); diff --git a/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts b/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts index fc093a5f5e0..e8ff49d9891 100644 --- a/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts +++ b/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts @@ -25,7 +25,8 @@ export const useIsHorizontalScrollingSupported = (): boolean => { useEffect(() => { if (renderingTarget === 'Apps') { - hasMinimumBridgetVersion('8.1.1').then( + // TODO: update version after new bridget patch is published + hasMinimumBridgetVersion('8.1.0').then( setHorizontalScrollingSupported, ); } From 3a0071dce1f5123480a2b329be46e2e9ddfafb27 Mon Sep 17 00:00:00 2001 From: Marjan Kalanaki Date: Fri, 13 Dec 2024 12:59:07 +0000 Subject: [PATCH 3/6] fix lint issues --- dotcom-rendering/src/components/Carousel.importable.tsx | 4 ++-- .../src/components/FetchOnwardsData.importable.tsx | 2 +- dotcom-rendering/src/components/OnwardsUpper.importable.tsx | 2 +- dotcom-rendering/src/layouts/AudioLayout.tsx | 2 +- dotcom-rendering/src/layouts/FrontLayout.tsx | 2 +- dotcom-rendering/src/layouts/NewsletterSignupLayout.tsx | 2 +- dotcom-rendering/src/lib/useIsBridgetCompatible.ts | 2 +- dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dotcom-rendering/src/components/Carousel.importable.tsx b/dotcom-rendering/src/components/Carousel.importable.tsx index 3fac3f6d446..dce17b68c17 100644 --- a/dotcom-rendering/src/components/Carousel.importable.tsx +++ b/dotcom-rendering/src/components/Carousel.importable.tsx @@ -9,6 +9,7 @@ import { import libDebounce from 'lodash.debounce'; import { useEffect, useRef, useState } from 'react'; import { ArticleDesign, type ArticleFormat } from '../lib/articleFormat'; +import { getInteractionClient } from '../lib/bridgetApi'; import { formatAttrString } from '../lib/formatAttrString'; import { getSourceImageUrl } from '../lib/getSourceImageUrl_temp_fix'; import { getZIndex } from '../lib/getZIndex'; @@ -24,6 +25,7 @@ import type { import type { LeftColSize } from '../types/layout'; import type { MainMedia } from '../types/mainMedia'; import type { OnwardsSource } from '../types/onwards'; +import type { RenderingTarget } from '../types/renderingTarget'; import type { TrailType } from '../types/trails'; import { Card } from './Card/Card'; import { LI } from './Card/components/LI'; @@ -32,8 +34,6 @@ import { ContainerOverrides } from './ContainerOverrides'; import { FormatBoundary } from './FormatBoundary'; import { Hide } from './Hide'; import { LeftColumn } from './LeftColumn'; -import { getInteractionClient } from '../lib/bridgetApi'; -import { RenderingTarget } from 'src/types/renderingTarget'; type Props = { heading: string; diff --git a/dotcom-rendering/src/components/FetchOnwardsData.importable.tsx b/dotcom-rendering/src/components/FetchOnwardsData.importable.tsx index 2f3d7f9823a..79d8dd751dd 100644 --- a/dotcom-rendering/src/components/FetchOnwardsData.importable.tsx +++ b/dotcom-rendering/src/components/FetchOnwardsData.importable.tsx @@ -6,10 +6,10 @@ import { useApi } from '../lib/useApi'; import { addDiscussionIds } from '../lib/useCommentCount'; import { palette } from '../palette'; import type { OnwardsSource } from '../types/onwards'; +import type { RenderingTarget } from '../types/renderingTarget'; import type { FETrailType, TrailType } from '../types/trails'; import { Carousel } from './Carousel.importable'; import { Placeholder } from './Placeholder'; -import { RenderingTarget } from 'src/types/renderingTarget'; type Props = { url: string; diff --git a/dotcom-rendering/src/components/OnwardsUpper.importable.tsx b/dotcom-rendering/src/components/OnwardsUpper.importable.tsx index a1e115ea827..47bbeefee05 100644 --- a/dotcom-rendering/src/components/OnwardsUpper.importable.tsx +++ b/dotcom-rendering/src/components/OnwardsUpper.importable.tsx @@ -9,10 +9,10 @@ import type { EditionId } from '../lib/edition'; import { useIsHorizontalScrollingSupported } from '../lib/useIsHorizontalScrollingSupported'; import { palette } from '../palette'; import type { OnwardsSource } from '../types/onwards'; +import type { RenderingTarget } from '../types/renderingTarget'; import type { TagType } from '../types/tag'; import { FetchOnwardsData } from './FetchOnwardsData.importable'; import { Section } from './Section'; -import { RenderingTarget } from 'src/types/renderingTarget'; type PillarForContainer = | 'headlines' diff --git a/dotcom-rendering/src/layouts/AudioLayout.tsx b/dotcom-rendering/src/layouts/AudioLayout.tsx index 76b3c04400e..e0674b71733 100644 --- a/dotcom-rendering/src/layouts/AudioLayout.tsx +++ b/dotcom-rendering/src/layouts/AudioLayout.tsx @@ -43,8 +43,8 @@ import { parse } from '../lib/slot-machine-flags'; import type { NavType } from '../model/extract-nav'; import { palette as themePalette } from '../palette'; import type { ArticleDeprecated } from '../types/article'; +import type { RenderingTarget } from '../types/renderingTarget'; import { BannerWrapper, Stuck } from './lib/stickiness'; -import { RenderingTarget } from '../types/renderingTarget'; const AudioGrid = ({ children }: { children: React.ReactNode }) => (
{ - hasMinimumBridgetVersion(requiredVersion).then(setIsCompatible); + void hasMinimumBridgetVersion(requiredVersion).then(setIsCompatible); }, [requiredVersion]); return isCompatible; }; diff --git a/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts b/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts index e8ff49d9891..1b9e8980cce 100644 --- a/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts +++ b/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts @@ -26,7 +26,7 @@ export const useIsHorizontalScrollingSupported = (): boolean => { useEffect(() => { if (renderingTarget === 'Apps') { // TODO: update version after new bridget patch is published - hasMinimumBridgetVersion('8.1.0').then( + void hasMinimumBridgetVersion('8.1.0').then( setHorizontalScrollingSupported, ); } From 50f04a8631df7085953d67f57c8f01c2f7edd382 Mon Sep 17 00:00:00 2001 From: Marjan Kalanaki Date: Fri, 13 Dec 2024 14:41:17 +0000 Subject: [PATCH 4/6] remove the check for android webview in reader revenue banner --- .../src/components/StickyBottomBanner.importable.tsx | 6 ------ .../components/StickyBottomBanner/ReaderRevenueBanner.tsx | 7 ------- 2 files changed, 13 deletions(-) diff --git a/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx b/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx index e7610261441..0399105510b 100644 --- a/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx +++ b/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx @@ -100,7 +100,6 @@ const buildRRBannerConfigWith = ({ tags, contributionsServiceUrl, idApiUrl, - isAndroidWebview, }: { isSignedIn: boolean; countryCode: CountryCode; @@ -116,7 +115,6 @@ const buildRRBannerConfigWith = ({ tags: TagType[]; contributionsServiceUrl: string; idApiUrl: string; - isAndroidWebview: boolean; }): CandidateConfig => { return { candidate: { @@ -151,7 +149,6 @@ const buildRRBannerConfigWith = ({ idApiUrl, signInGateWillShow, asyncArticleCounts, - isAndroidWebview, }), show: ({ meta, module, fetchEmail }: BannerProps) => @@ -245,7 +242,6 @@ export const StickyBottomBanner = ({ isPreview, currentLocaleCode: countryCode, }); - const isAndroidWebview = false; // TODO: fix in upcoming commit !!useIsAndroid(); useEffect(() => { setAsyncArticleCounts(getArticleCounts(pageId, tags, contentType)); @@ -282,7 +278,6 @@ export const StickyBottomBanner = ({ tags, contributionsServiceUrl, idApiUrl, - isAndroidWebview, }); const brazeArticleContext: BrazeArticleContext = { section: sectionId, @@ -316,7 +311,6 @@ export const StickyBottomBanner = ({ contentType, contributionsServiceUrl, idApiUrl, - isAndroidWebview, isMinuteArticle, isPaidContent, isPreview, diff --git a/dotcom-rendering/src/components/StickyBottomBanner/ReaderRevenueBanner.tsx b/dotcom-rendering/src/components/StickyBottomBanner/ReaderRevenueBanner.tsx index b0828463e2a..992ddb18c48 100644 --- a/dotcom-rendering/src/components/StickyBottomBanner/ReaderRevenueBanner.tsx +++ b/dotcom-rendering/src/components/StickyBottomBanner/ReaderRevenueBanner.tsx @@ -62,7 +62,6 @@ type CanShowProps = BaseProps & { idApiUrl: string; signInGateWillShow: boolean; asyncArticleCounts: Promise; - isAndroidWebview: boolean; }; type ReaderRevenueComponentType = @@ -192,15 +191,9 @@ export const canShowRRBanner: CanShowFunctionType = async ({ idApiUrl, signInGateWillShow, asyncArticleCounts, - isAndroidWebview, }) => { if (!remoteBannerConfig) return { show: false }; - if (isAndroidWebview) { - // Do not show banners on Android app webview, due to buggy behaviour with the buttons - return { show: false }; - } - if ( shouldHideReaderRevenue || isPaidContent || From 1daf088e7f14ea34341a5000f73fda824ca1042a Mon Sep 17 00:00:00 2001 From: Marjan Kalanaki Date: Mon, 13 Jan 2025 17:37:44 +0000 Subject: [PATCH 5/6] use right bridget version to determine if horizontal scrolling is supported --- dotcom-rendering/src/components/OnwardsUpper.importable.tsx | 1 - dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/dotcom-rendering/src/components/OnwardsUpper.importable.tsx b/dotcom-rendering/src/components/OnwardsUpper.importable.tsx index 47bbeefee05..ea70f4b31f2 100644 --- a/dotcom-rendering/src/components/OnwardsUpper.importable.tsx +++ b/dotcom-rendering/src/components/OnwardsUpper.importable.tsx @@ -217,7 +217,6 @@ export const OnwardsUpper = ({ absoluteServerTimes, renderingTarget, }: Props) => { - const isHorizontalScrollingSupported = useIsHorizontalScrollingSupported(); if (!isHorizontalScrollingSupported) return null; diff --git a/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts b/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts index 1b9e8980cce..8b8123f47c5 100644 --- a/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts +++ b/dotcom-rendering/src/lib/useIsHorizontalScrollingSupported.ts @@ -26,7 +26,7 @@ export const useIsHorizontalScrollingSupported = (): boolean => { useEffect(() => { if (renderingTarget === 'Apps') { // TODO: update version after new bridget patch is published - void hasMinimumBridgetVersion('8.1.0').then( + void hasMinimumBridgetVersion('8.3.2').then( setHorizontalScrollingSupported, ); } From 123a5a728b18eba490d6160b04da285a8f164054 Mon Sep 17 00:00:00 2001 From: Marjan Kalanaki Date: Wed, 22 Jan 2025 15:33:40 +0000 Subject: [PATCH 6/6] fix prettier issue --- dotcom-rendering/src/layouts/FrontLayout.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dotcom-rendering/src/layouts/FrontLayout.tsx b/dotcom-rendering/src/layouts/FrontLayout.tsx index 5450789e866..ab2a4987be8 100644 --- a/dotcom-rendering/src/layouts/FrontLayout.tsx +++ b/dotcom-rendering/src/layouts/FrontLayout.tsx @@ -621,7 +621,9 @@ export const FrontLayout = ({ front, NAV }: Props) => { absoluteServerTimes={ absoluteServerTimes } - renderingTarget={renderingTarget} + renderingTarget={ + renderingTarget + } />