Fix: Offline message not displayed when opening video in Chat while offline#89786
Fix: Offline message not displayed when opening video in Chat while offline#89786nabi-ebrahimi wants to merge 8 commits into
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
@Ollyws, friendly bump. thanks. |
|
Will get to this asap. |
| // Skip API root normalization for search attachments because this route is only opened from preview, | ||
| // which already passes a resolved source. Keep normalization for other types to support email entry points. | ||
| const source = getValidatedImageSource(sourceParam, type !== CONST.ATTACHMENT_TYPE.SEARCH); | ||
| const fileName = originalFileName ?? (typeof source === 'string' ? getFileName(source) : ''); |
There was a problem hiding this comment.
Could you explain this line?
There was a problem hiding this comment.
We need this because the modal offline-video check was not working reliably when originalFileName was missing. AttachmentModalBaseContent uses this value to create file.name, and AttachmentView uses file.name to detect video attachments. Without this fallback, the modal can fail to recognize the attachment as a video and skip the offline handling.
| const attrHref = htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE] || htmlAttribs.src || htmlAttribs.href || ''; | ||
| const sourceURL = tryResolveUrlFromApiRoot(attrHref); | ||
| const fileName = getFileName(`${sourceURL}`); | ||
| const fileName = htmlAttribs[CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE] || getFileName(`${sourceURL}`); |
There was a problem hiding this comment.
We need this because the route was sometimes passing a filename derived from an opaque attachment URL, so the modal could fail to detect the attachment as a video. The actual uploaded filename is available in the HTML data-name attribute, so passing that as originalFileName makes the modal’s video/offline logic reliable.
|
@Ollyws I need one clarification. This PR currently fixes the attachment modal behavior so the offline view is shown when opening a video while offline. However, in the chat view itself, the same video can still be played while offline from the preview. Should this PR also block offline video playback directly in chat, or is the expected scope only the attachment modal? Thanks REC-20260514103200.mp4 |
|
@Ollyws, gentle bump. thanks. |
|
@nabi-ebrahimi Yeah I think it would be best if the same behaviour applied to the preview, i.e it's disabled using the same conditions. |
|
@Ollyws I’ve updated this as well, so the offline indicator is now shown in the chat preview using the same conditions as the modal. Could you please take another look when you get a chance? Thanks! |
|
@Ollyws, gentle bump. thanks. |
| accessibilityLabel={fileName} | ||
| isDeleted={isDeleted} | ||
| /> | ||
| {shouldShowOfflineVideoIndicator && <AttachmentOfflineIndicator isPreview />} |
There was a problem hiding this comment.
I think this would be better inside the VideoPlayerThumbnail component, as let's make sure it retains the same style, as now it just sits over the top of the image and doesn't have the same border:
2 participants
Explanation of Change
These changes prevent remote video attachments from trying to load while the user is offline. Instead, the attachment modal shows an offline blocking view, hides attachment actions like download and the three-dot menu, and avoids rendering the video content until connectivity is restored.
This gives users clearer feedback when a video cannot be accessed offline, instead of showing a broken or stuck attachment preview.
Fixed Issues
$ #88216
PROPOSAL: #88216 (comment)
Tests
Offline tests
Same as Tests.
QA Steps
Same as Tests.
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-05-07.at.2.00.10.AM.mov
Android: mWeb Chrome
Screen.Recording.2026-05-06.at.11.52.35.PM.mov
iOS: Native
Screen.Recording.2026-05-07.at.12.08.51.PM.mov
iOS: mWeb Safari
Screen.Recording.2026-05-11.at.1.56.07.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2026-05-07.at.11.59.10.AM.mov