Remove useIsAndroid Hook and add Support for Horizontal Scrolling #12987
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +737 B (+0.08%) Total Size: 885 kB
ℹ️ View Unchanged
|
marjisound
force-pushed
the
optimize-useIsAndroid-hook
branch
2 times, most recently
from
61568f3 to
18a0aa9
Compare
marjisound
changed the title
marjisound
changed the title
github-actions
bot
removed
the
run_chromatic
|
"This PR is stale because it has been open 30 days with no activity. Unless a comment is added or the “stale” label removed, this will be closed in 3 days" |
marjisound
force-pushed
the
optimize-useIsAndroid-hook
branch
2 times, most recently
from
b24e2b4 to
37249ee
Compare
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
marjisound
force-pushed
the
optimize-useIsAndroid-hook
branch
from
37249ee to
6436021
Compare
github-actions
bot
removed
the
run_chromatic
SiAdcock
approved these changes
Jan 21, 2025
Co-authored-by: Jamie B <[email protected]>
marjisound
force-pushed
the
optimize-useIsAndroid-hook
branch
from
6436021 to
2c98164
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Co-authored-by: Jamie B [email protected]
DO NOT MERGE UNLESS BRIDGET 8.3.2 IS IN BOTH IOS & ANDROID PROD
Or you can check if https://github.com/guardian/android-news-app/pull/11166 gets label
released_to_productionWhat does this change?
Summary
This PR removes the useIsAndroid hook from DCR. The hook was previously used for two main purposes:
Reader Revenue Banner
The useIsAndroid hook was used in the StickyBottomBanner.importable component. However, this component is only rendered for the web rendering target, and the hook always returns false in this context. As a result, the hook was effectively redundant and did not impact the behavior of the banner.
Carousel
The useIsAndroid hook was used in the Carousel.importable and OnwardsUpper.importable components to prevent these components from rendering on Android. This was necessary because the carousel's horizontal scrolling conflicted with the Android article swipe functionality within the WebView.
Android implementation for supporting horizontal scrolling
To resolve this, the Android team implemented a change in the native layer. The article swiping functionality is now disabled when the
disableArticleSwipeBridget method is called with the argument true, and re-enabled when called with false.This allows article swiping to be disabled when interacting with the carousel. Details of this implementation can be found here: PR #10947.
New Implementation:
This PR introduces a new hook:
useIsHorizontalScrollingSupported. The hook determines whether horizontal scrolling is supported based on:Rendering Target & Bridget version: Returns true if the target is Apps and bridget version is compatible
User Agent Check: If it's Apps and the Bridget version is not compatible, it falls back to checking if the user agent indicates Android, returning false for Android devices.
Over time, as more users migrate to the compatible Bridget version, the reliance on the user agent check will diminish, reducing the overhead of this fallback.
Additional Changes for Carousel on Android
To support carousel rendering on Android
onTouchStartandonTouchEndevent handlers are added. These notify the native layer to disable or enable article swiping when a user interacts with the carousel.Why?
We want to remove any reliance on which platform (ios or android) the article is being rendered for. DCR should only care if it's rendering for web or app.
Screenshots
Screen_recording_20250113_173528.mp4
Screen_recording_20250113_173135.mp4