Skip to content

fix: attachment preview header layout issues - WPB-23931#4491

Open
WilhelmOks wants to merge 2 commits intorelease/cycle-4.17from
fix/WPB-23931-large-attachment-preview-header-padding
Open

fix: attachment preview header layout issues - WPB-23931#4491
WilhelmOks wants to merge 2 commits intorelease/cycle-4.17from
fix/WPB-23931-large-attachment-preview-header-padding

Conversation

@WilhelmOks
Copy link
Collaborator

@WilhelmOks WilhelmOks commented Mar 25, 2026

BugWPB-23931 [iOS][Files in conversation] Remove top padding from video and files thumbnails

Issue

In the file attachment header, the space between the first line (file icon and file type) and the second line (file name) was too large.
The issue was that in many places the size was set to take up maximum space, either through maxHeight: .infinity or through Spacer(). Then to reduce the height, an explicit height was set (height: 74).
This not only caused the layout issues but also broke the UI when the font size was set to very large.
I fixed it by removing the indefinite growing and the explicit heights.

Additionally I added a gray background and some padding in SwiftUI Previews to better see if there are issues with the layout.

Testing

Send messages with attachments of different files and check if the header layout is ok.
Especially the space between the file type and the file name should not be too large.
Most important to check are video files and documents.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 25, 2026

Test Results

266 tests   265 ✅  31s ⏱️
 53 suites    0 💤
  1 files      1 ❌

For more details on these failures, see this check.

Results for commit 5f4fc38.

♻️ This comment has been updated with latest results.

Summary: workflow run #23546286259
Allure report (download zip): html-report-28798-fix_WPB-23931-large-attachment-preview-header-padding

@datadog-wireapp
Copy link

datadog-wireapp bot commented Mar 25, 2026

⚠️ Tests

Fix all issues with Cursor

⚠️ Warnings

🧪 1 Test failed

testConfigurationVariations() from WireMessagingTests.WireDriveDocumentAttachmentPreviewTests   View in Datadog   (Fix with Cursor)
WireDriveDocumentAttachmentPreviewTests.swift:82: XCTAssertNil failed: "Snapshot "2.dark" does not match reference.

@−
"file:///Users/admin/actions-runner/_work/wire-ios/wire-ios/WireMessaging/Tests/WireMessagingTests/Resources/ReferenceImages/WireDriveDocumentAttachmentPreviewTests/testConfigurationVariations.2-dark.png"
@+
"file:///Users/admin/actions-runner/_work/wire-ios/wire-ios/WireMessaging/Tests/WireMessagingTests/Resources/SnapshotResults/WireDriveDocumentAttachmentPreviewTests/testConfigurationVariations.2-dark.png"

To configure output for a custom diff tool, use 'withSnapshotTesting'. For example:

    withSnapshotTesting(diffTool: .ksdiff) {
...

ℹ️ Info

No other issues found (see more)

❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 5f4fc38 | Docs | Was this helpful? Give us feedback!

Copy link
Contributor

@jullianm jullianm left a comment

Choose a reason for hiding this comment

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

It does fix the issue in conversation previews however it also changes the layout (height) of draft previews which might be acceptable but I think it's better to keep the draft previews as it was before

@sonarqubecloud
Copy link

@WilhelmOks
Copy link
Collaborator Author

WilhelmOks commented Mar 25, 2026

It does fix the issue in conversation previews however it also changes the layout (height) of draft previews which might be acceptable but I think it's better to keep the draft previews as it was before

In the latest commit I changed it for the draft previews to keep the same bubble height as the other elements in the list

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants