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.

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

Remove useIsAndroid Hook and add Support for Horizontal Scrolling #12987

Merged
merged 6 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 19 additions & 3 deletions dotcom-rendering/src/components/Carousel.importable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ 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';
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';
Expand All @@ -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';
Expand All @@ -42,6 +44,7 @@ type Props = {
leftColSize: LeftColSize;
discussionApiUrl: string;
absoluteServerTimes: boolean;
renderingTarget: RenderingTarget;
};

type ArticleProps = Props & {
Expand Down Expand Up @@ -792,13 +795,16 @@ export const Carousel = ({
discussionApiUrl,
isOnwardContent = true,
absoluteServerTimes,
renderingTarget,
...props
}: ArticleProps | FrontProps) => {
const carouselRef = useRef<HTMLUListElement>(null);

const [index, setIndex] = useState(0);
const [maxIndex, setMaxIndex] = useState(0);
const isAndroid = useIsAndroid();
const isHorizontalScrollingSupported = useIsHorizontalScrollingSupported();

const isApps = renderingTarget === 'Apps';

const arrowName = 'carousel-small-arrow';

Expand Down Expand Up @@ -907,7 +913,15 @@ export const Carousel = ({
// when index changes and compare it against the prior maxIndex only.
useEffect(() => setMaxIndex((m) => Math.max(index, m)), [index]);

if (isAndroid) {
const onTouchStart = async () => {
await getInteractionClient().disableArticleSwipe(true);
};

const onTouchEnd = async () => {
await getInteractionClient().disableArticleSwipe(false);
};

if (!isHorizontalScrollingSupported) {
return null;
}

Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions dotcom-rendering/src/components/Carousel.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ export const Headlines: StoryObj = ({ format }: StoryProps) => {
leftColSize="compact"
discussionApiUrl={discussionApiUrl}
absoluteServerTimes={true}
renderingTarget="Web"
/>
</Section>
);
Expand All @@ -295,6 +296,7 @@ export const SingleItemCarousel = () => {
leftColSize="compact"
discussionApiUrl={discussionApiUrl}
absoluteServerTimes={true}
renderingTarget="Web"
/>
</Section>
);
Expand Down Expand Up @@ -349,6 +351,7 @@ export const SingleOpinionCarousel = () => {
leftColSize="compact"
discussionApiUrl={discussionApiUrl}
absoluteServerTimes={true}
renderingTarget="Web"
/>
</Section>
);
Expand Down Expand Up @@ -379,6 +382,7 @@ export const Immersive = () => {
leftColSize="compact"
discussionApiUrl={discussionApiUrl}
absoluteServerTimes={true}
renderingTarget="Web"
/>
</Section>
</>
Expand Down Expand Up @@ -424,6 +428,7 @@ export const SpecialReportAlt = () => {
leftColSize="compact"
discussionApiUrl={discussionApiUrl}
absoluteServerTimes={true}
renderingTarget="Web"
/>
</Section>
);
Expand Down Expand Up @@ -639,6 +644,7 @@ export const AllCards = () => {
discussionApiUrl={discussionApiUrl}
format={defaultFormat}
absoluteServerTimes={true}
renderingTarget="Web"
/>
</Section>
);
Expand All @@ -663,6 +669,7 @@ export const FrontCarousel = () => (
discussionApiUrl={discussionApiUrl}
palette="PodcastPalette"
absoluteServerTimes={true}
renderingTarget="Web"
/>
</Section>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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';
Expand All @@ -17,6 +18,7 @@ type Props = {
format: ArticleFormat;
discussionApiUrl: string;
absoluteServerTimes: boolean;
renderingTarget: RenderingTarget;
};

type OnwardsResponse = {
Expand All @@ -37,6 +39,7 @@ export const FetchOnwardsData = ({
format,
discussionApiUrl,
absoluteServerTimes,
renderingTarget,
}: Props) => {
const { data, error } = useApi<OnwardsResponse>(url);

Expand Down Expand Up @@ -85,6 +88,7 @@ export const FetchOnwardsData = ({
}
discussionApiUrl={discussionApiUrl}
absoluteServerTimes={absoluteServerTimes}
renderingTarget={renderingTarget}
/>
</div>
);
Expand Down
1 change: 1 addition & 0 deletions dotcom-rendering/src/components/Island.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ describe('Island: server-side rendering', () => {
shortUrlId=""
discussionApiUrl=""
absoluteServerTimes={true}
renderingTarget="Web"
/>
</WithConfig>,
),
Expand Down
12 changes: 9 additions & 3 deletions dotcom-rendering/src/components/OnwardsUpper.importable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ 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 { RenderingTarget } from '../types/renderingTarget';
import type { TagType } from '../types/tag';
import { FetchOnwardsData } from './FetchOnwardsData.importable';
import { Section } from './Section';
Expand Down Expand Up @@ -179,6 +180,7 @@ type Props = {
shortUrlId: string;
discussionApiUrl: string;
absoluteServerTimes: boolean;
renderingTarget: RenderingTarget;
};

/**
Expand Down Expand Up @@ -213,9 +215,11 @@ export const OnwardsUpper = ({
shortUrlId,
discussionApiUrl,
absoluteServerTimes,
renderingTarget,
}: 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
Expand Down Expand Up @@ -309,6 +313,7 @@ export const OnwardsUpper = ({
format={format}
discussionApiUrl={discussionApiUrl}
absoluteServerTimes={absoluteServerTimes}
renderingTarget={renderingTarget}
/>
</Section>
)}
Expand All @@ -324,6 +329,7 @@ export const OnwardsUpper = ({
format={format}
discussionApiUrl={discussionApiUrl}
absoluteServerTimes={absoluteServerTimes}
renderingTarget={renderingTarget}
/>
</Section>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -101,7 +100,6 @@ const buildRRBannerConfigWith = ({
tags,
contributionsServiceUrl,
idApiUrl,
isAndroidWebview,
}: {
isSignedIn: boolean;
countryCode: CountryCode;
Expand All @@ -117,7 +115,6 @@ const buildRRBannerConfigWith = ({
tags: TagType[];
contributionsServiceUrl: string;
idApiUrl: string;
isAndroidWebview: boolean;
}): CandidateConfig<BannerProps> => {
return {
candidate: {
Expand Down Expand Up @@ -152,7 +149,6 @@ const buildRRBannerConfigWith = ({
idApiUrl,
signInGateWillShow,
asyncArticleCounts,
isAndroidWebview,
}),
show:
({ meta, module, fetchEmail }: BannerProps) =>
Expand Down Expand Up @@ -246,7 +242,6 @@ export const StickyBottomBanner = ({
isPreview,
currentLocaleCode: countryCode,
});
const isAndroidWebview = !!useIsAndroid();

useEffect(() => {
setAsyncArticleCounts(getArticleCounts(pageId, tags, contentType));
Expand Down Expand Up @@ -283,7 +278,6 @@ export const StickyBottomBanner = ({
tags,
contributionsServiceUrl,
idApiUrl,
isAndroidWebview,
});
const brazeArticleContext: BrazeArticleContext = {
section: sectionId,
Expand Down Expand Up @@ -317,7 +311,6 @@ export const StickyBottomBanner = ({
contentType,
contributionsServiceUrl,
idApiUrl,
isAndroidWebview,
isMinuteArticle,
isPaidContent,
isPreview,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ type CanShowProps = BaseProps & {
idApiUrl: string;
signInGateWillShow: boolean;
asyncArticleCounts: Promise<ArticleCounts | undefined>;
isAndroidWebview: boolean;
};

type ReaderRevenueComponentType =
Expand Down Expand Up @@ -192,15 +191,9 @@ export const canShowRRBanner: CanShowFunctionType<BannerProps> = 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 ||
Expand Down
7 changes: 6 additions & 1 deletion dotcom-rendering/src/layouts/AudioLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ 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';

const AudioGrid = ({ children }: { children: React.ReactNode }) => (
Expand Down Expand Up @@ -124,14 +125,16 @@ const maxWidth = css`
interface Props {
article: ArticleDeprecated;
format: ArticleFormat;
renderingTarget: RenderingTarget;
}

interface WebProps extends Props {
NAV: NavType;
renderingTarget: 'Web';
}

export const AudioLayout = (props: WebProps) => {
const { article, format } = props;
const { article, format, renderingTarget } = props;
const audioData = getAudioData(article.mainMediaElements);

const {
Expand Down Expand Up @@ -495,6 +498,7 @@ export const AudioLayout = (props: WebProps) => {
article.config.discussionApiUrl
}
absoluteServerTimes={absoluteServerTimes}
renderingTarget={renderingTarget}
/>
</Island>
</Section>
Expand All @@ -518,6 +522,7 @@ export const AudioLayout = (props: WebProps) => {
shortUrlId={article.config.shortUrlId}
discussionApiUrl={article.config.discussionApiUrl}
absoluteServerTimes={absoluteServerTimes}
renderingTarget={renderingTarget}
/>
</Island>
{showComments && (
Expand Down
2 changes: 2 additions & 0 deletions dotcom-rendering/src/layouts/CommentLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
article.config.discussionApiUrl
}
absoluteServerTimes={absoluteServerTimes}
renderingTarget={renderingTarget}
/>
</Island>
</Section>
Expand All @@ -760,6 +761,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
shortUrlId={article.config.shortUrlId}
discussionApiUrl={article.config.discussionApiUrl}
absoluteServerTimes={absoluteServerTimes}
renderingTarget={renderingTarget}
/>
</Island>

Expand Down
2 changes: 2 additions & 0 deletions dotcom-rendering/src/layouts/DecideLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ const DecideLayoutWeb = ({
article={article}
NAV={NAV}
format={format}
renderingTarget={renderingTarget}
/>
);
case ArticleDesign.Audio:
Expand All @@ -271,6 +272,7 @@ const DecideLayoutWeb = ({
article={article}
format={format}
NAV={NAV}
renderingTarget={renderingTarget}
/>
);
default:
Expand Down
6 changes: 6 additions & 0 deletions dotcom-rendering/src/layouts/FrontLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from '@guardian/source/foundations';
import { AdSlot } from '../components/AdSlot.web';
import { Carousel } from '../components/Carousel.importable';
import { useConfig } from '../components/ConfigContext';
import { ContainerOverrides } from '../components/ContainerOverrides';
import { CPScottHeader } from '../components/CPScottHeader';
import { DecideContainer } from '../components/DecideContainer';
Expand Down Expand Up @@ -114,6 +115,8 @@ export const FrontLayout = ({ front, NAV }: Props) => {
editionId,
} = front;

const { renderingTarget } = useConfig();

const renderAds = canRenderAds(front);

const hasPageSkin = renderAds && hasPageSkinConfig;
Expand Down Expand Up @@ -618,6 +621,9 @@ export const FrontLayout = ({ front, NAV }: Props) => {
absoluteServerTimes={
absoluteServerTimes
}
renderingTarget={
renderingTarget
}
/>
</Island>
</Section>
Expand Down
Loading
Loading