Skip to content

Conversation

@brainbicycle
Copy link
Contributor

@brainbicycle brainbicycle commented Oct 15, 2025

This PR resolves DIAM-129

Description

Adds basic video header support for native editorial articles.
Embeds the video in a webview, the same approach we use for embedded videos elsewhere in articles.
Draft until tested for performance and design sign off.

Platform Before After
Android android before
android-video-header-low.mov
iOS iphone before
video-header-low.mov

PR Checklist

  • I have tested my changes on the following platforms:
    • Android.
    • iOS.
  • I hid my changes behind a feature flag, or they don't need one.
  • I have included screenshots or videos at least on Android, or I have not changed the UI.
  • I have added tests, or my changes don't require any.
  • I added an app state migration, or my changes do not require one.
  • I have documented any follow-up work that this PR will require, or it does not require any.
  • I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

  • I would like at least one of the reviewers to run this PR on the simulator or device.
Changelog updates

Changelog updates

Cross-platform user-facing changes

  • add support for video headers in native editorial articles - brian

iOS user-facing changes

Android user-facing changes

Dev changes

Need help with something? Have a look at our docs, or get in touch with us.

brainbicycle and others added 5 commits October 15, 2025 11:41
Update ArticleHero component to match web implementation for video heroes:
- Position text overlay absolutely on top of video using LinearGradient
- Add gradient background (transparent to rgba(0,0,0,0.6)) for readability
- Style text in white with subtle shadows (15px blur, 0.25 opacity)
- Maintain existing behavior for image heroes (text below image)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Updates article hero video to match web implementation where video height
is calculated as max(50vh - navHeight, 360px) instead of using aspect ratio.
This ensures consistent behavior with the web FULLSCREEN layout, where the
video fills a height-constrained container with object-fit: cover.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@brainbicycle brainbicycle changed the title Brian/editorial video feat: support video headers in editorial Oct 15, 2025
@brainbicycle brainbicycle marked this pull request as draft October 15, 2025 22:37
@brainbicycle brainbicycle self-assigned this Oct 15, 2025
@artsy artsy deleted a comment from ArtsyOpenSource Oct 15, 2025
brainbicycle and others added 6 commits October 15, 2025 20:27
- Added react-native-linear-gradient mock to setupJest.tsx so it's available
  globally for all tests
- Updated ArticleHero tests to explicitly set media: null in test data to
  ensure image rendering path is tested instead of video path
- Removed local LinearGradient mock from ArticleHero tests since it's now global

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Changed video preload from "metadata" to "auto" for faster loading
- Enabled WebView caching (cacheEnabled: true, cacheMode: "LOAD_DEFAULT")
- Added mixedContentMode="always" for better Android network performance
- Added Android-specific optimizations (javaScriptEnabled, domStorageEnabled, allowFileAccess)
- Added testID to ArticleHeroVideo component for testing
- Added test for video rendering
- Fixed WebView mock to export both default and named WebView export

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@brainbicycle brainbicycle marked this pull request as ready for review October 16, 2025 16:51
@ArtsyOpenSource
Copy link
Contributor

This PR contains the following changes:

  • Cross-platform user-facing changes (add support for video headers in native editorial articles - brian - brainbicycle)

Generated by 🚫 dangerJS against c565e18

@brainbicycle brainbicycle marked this pull request as draft October 16, 2025 17:16
return <View ref={ref} {...props} />
}),
default: MockWebView,
WebView: MockWebView,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

missing named export mock was causing test failures

</body>
</html>
`,
[videoUrl, backgroundColor]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

using embedded html and webviews built in video playback capabilities, customization is limited and performance specifically on Android is not great (at least on my old test device), but is what we are currently using for embedded videos, if we find it is not up to the task we should explore video libraries like react-native-video

Copy link
Member

@gkartalis gkartalis left a comment

Choose a reason for hiding this comment

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

noice!

Copy link
Member

@MounirDhahri MounirDhahri left a comment

Choose a reason for hiding this comment

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

Nice! clean

@brainbicycle brainbicycle marked this pull request as ready for review October 17, 2025 15:58
@brainbicycle brainbicycle merged commit 784b325 into main Oct 17, 2025
9 checks passed
@brainbicycle brainbicycle deleted the brian/editorial-video branch October 17, 2025 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants