Skip to content

Disable app article swipe on interactive atom blocks #14131

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Disable app article swipe on interactive atom blocks #14131

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
20d00e3
Disable touch events on interactive atom blocks
frederickobrien Jun 25, 2025
e0becc5
Update dotcom-rendering/src/components/InteractiveBlockComponent.impo…
frederickobrien Jun 25, 2025
1398efb
Update dotcom-rendering/src/components/InteractiveBlockComponent.impo…
frederickobrien Jun 25, 2025
2e7fcf5
Fix boolean check mistake
frederickobrien Jun 25, 2025
0c0e376
Add to `InteractiveAtom`
frederickobrien Aug 12, 2025
d94e917
Remove from InteractiveBlockComponent
frederickobrien Sep 19, 2025
a7edc7c
Initiate functions once, wrap component in `Island`
frederickobrien Sep 19, 2025
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
8 changes: 8 additions & 0 deletions dotcom-rendering/src/components/InteractiveAtom.tsx
View file
Open in desktop
Copy link
Contributor

@JamieB-gu JamieB-gu Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description above says (emphasis mine):

As per #14140 the changes here currently apply to interactive elements but not atoms.

But this is the component for interactive atoms?

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { css } from '@emotion/react';
import { getInteractionClient } from '../lib/bridgetApi';
import { unifyPageContent } from '../lib/unifyPageContent';
import { palette } from '../palette';
import { useConfig } from './ConfigContext';
Expand Down Expand Up @@ -47,11 +48,18 @@ export const InteractiveAtom = ({
}: InteractiveAtomType) => {
const { renderingTarget } = useConfig();

const isApps = renderingTarget === 'Apps';

const onTouchStart = () => getInteractionClient().disableArticleSwipe(true);
const onTouchEnd = () => getInteractionClient().disableArticleSwipe(false);

return (
<div
css={[containerStyles, !!isMainMedia && fullHeightStyles]}
data-atom-id={id}
data-atom-type="interactive"
onTouchStart={isApps ? onTouchStart : undefined}
onTouchEnd={isApps ? onTouchEnd : undefined}
Copy link
Contributor

@JamieB-gu JamieB-gu Sep 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't appear to be wrapped in an Island, so what allows this code to run on the client?

>
<Island
priority="feature"
Expand Down
9 changes: 9 additions & 0 deletions dotcom-rendering/src/components/InteractiveBlockComponent.importable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import libDebounce from 'lodash.debounce';
import { useRef, useState } from 'react';
import { interactiveLegacyFigureClasses } from '../layouts/lib/interactiveLegacyStyling';
import { type ArticleFormat, ArticleSpecial } from '../lib/articleFormat';
import { getInteractionClient } from '../lib/bridgetApi';
import { useOnce } from '../lib/useOnce';
import { palette as themePalette } from '../palette';
import type { RoleType } from '../types/content';
Expand Down Expand Up @@ -349,6 +350,12 @@ export const InteractiveBlockComponent = ({
? true
: false;

const isApps = renderingTarget === 'Apps';

const onTouchStart = () => getInteractionClient().disableArticleSwipe(true);

const onTouchEnd = () => getInteractionClient().disableArticleSwipe(false);
Copy link
Member

@arelra arelra Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small performance suggestion.

I think these functions will be created on every render of this component even though they may not be used by the web rendering target. We could inline them in but again they will be created on every render of the component. useCallback is an option but given the functions are idempotent and afaics don't rely on any state could they be moved outside of the component to the module level?


useOnce(() => {
// We've brought the behavior from boot.js into this file to avoid loading 2 extra scripts

Expand Down Expand Up @@ -455,6 +462,8 @@ export const InteractiveBlockComponent = ({
data-alt={alt} // for compatibility with custom boot scripts
data-testid={`interactive-element-${encodeURI(alt ?? '')}`}
data-spacefinder-role={role}
onTouchStart={isApps ? onTouchStart : undefined}
onTouchEnd={isApps ? onTouchEnd : undefined}
>
{!loaded && (
<>
Expand Down