feat: wire cells upload images & videos previews - WPB-15844 #2513

El-Fitz · 2025-02-11T13:12:08Z

WPB-15844 [iOS] Create upload preview component for images

Issue

Implements upload previews for image (WPB-15844) and videos (WPB-15845).
Accessibility labels will be added when available.

Testing

See UI Tests snapshots.

Checklist

Title contains a reference JIRA issue number like [WPB-XXX].
Description is filled and free of optional paragraphs.
Adds/updates automated tests.

UI accessibility checklist

If your PR includes UI changes, please utilize this checklist:

Make sure you use the API for UI elements that support large fonts.
All colors are taken from WireDesign.ColorTheme or constructed using WireDesign.BaseColorPalette.

github-actions · 2025-02-11T13:19:38Z

Test Results

1 files 10 suites 56s ⏱️
133 tests 131 ✅ 1 💤 1 ❌
133 runs 132 ✅ 1 💤 0 ❌

For more details on these failures, see this check.

Results for commit ebd2bd3.

♻️ This comment has been updated with latest results.

caldrian

I think we should have the schemes WireDebugAll and WireCellsAll, which CI will use to run tests.
I can do that, if you prefer.

caldrian · 2025-02-11T13:20:33Z

WireCells/Package.swift

It would be nice to have the upcoming features enabled, like other packages do, see WireFoundation please.

This package builds with the 6.0 toolchain, so those are not upcoming features and are already enabled for this package.

I disagree:
ExistentialAny
InternalImportsByDefault
FullTypedThrows

made the changes

Then we should update WireAuthentication/Package.swift in a separate PR

for target in package.targets { if target.isTest { target.dependencies += [WireTestingPackage] } target.swiftSettings = (target.swiftSettings ?? []) + [ // TODO: [WPB-15967] Enable `ExistentialAny` upcoming feature .enableUpcomingFeature("GlobalConcurrency"), .enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-enable-bare-slash-regex"]) // For regex literals ] }

WireDebug/Package.swift

El-Fitz · 2025-02-11T14:09:53Z

I think we should have the schemes WireDebugAll and WireCellsAll, which CI will use to run tests. I can do that, if you prefer.

Oh. Yes, please do. But it doesn’t make sense for WireDebug.

datadog-wireapp · 2025-02-11T16:18:21Z

Datadog Report

Branch report: feat/wire-cells-upload-preview-images
Commit report: 855cce7
Test service: wire-ios-mono

✅ 0 Failed, 98 Passed, 1 Skipped, 39.86s Total Time

WireCells/Sources/WireCellsUI/Components/LocalImagePreview.swift

samwyndham

Looking good! I think it would make sense for the snapshots to include the dismiss button

samwyndham · 2025-02-12T08:05:52Z

WireCells/Tests/WireCellsUITests/Components/UploadVideoPreviewTests.swift

+    override func tearDown() {
+        snapshotHelper = nil
+    }
+


suggestion (non-blocking): Add a snapshot test to show the different duration format variations

...Resources/ReferenceImages/UploadImagePreviewTests/testColorSchemeVariantsEmptyState.dark.png

netbe

I left some comments about the setup mostly

fastlane/framework.rb

wire-ios-build-assets

...ReferenceImages/UploadImagePreviewTests/testDynamicTypeVariantsEmptyState.accessibility5.png

netbe

the submodule should be reverted to a1c04f

...Resources/ReferenceImages/UploadVideoPreviewTests/testColorSchemeVariantsEmptyState.dark.png

wire-ios-build-assets

netbe

🚢

caldrian · 2025-02-13T18:37:20Z

WireCells/Sources/WireCellsImplementation/WireCellsServiceImplementation.swift

-        let request = RestLookupRequest()
+        var request = RestLookupRequest()
        let locator = RestNodeLocator(path: path)
        request.locators = RestNodeLocators(many: [locator])
        let requestBuilder = NodeServiceAPI.lookupWithRequestBuilder(body: request, apiConfiguration: cellsApiConfig)

        do {
            let response: Response<RestNodeCollection> = try await requestBuilder.execute()
-            return response.body.nodes
+            return response.body.nodes ?? []


There were build errors which I fixed, please verify @El-Fitz

Yes, probably due to the change in the dependency version. They moved from a class to a struct. Thank you.

caldrian · 2025-02-13T18:37:31Z

WireCells/Package.swift

made the changes

caldrian · 2025-02-14T09:22:57Z

WireCells/Tests/TestPlans/AllTests.xctestplan

@@ -15,6 +15,13 @@
  },
  "testTargets" : [
    {
+      "skippedTests" : [
+        "WireCellsTests",


Why are the tests skipped?
How can we prevent to forget about them?

/// These tests are intended for locally testing the service, not for CI.

caldrian · 2025-02-14T09:26:21Z

fastlane/framework.rb

+        when "WireCellsAll"
+            name


Suggested change

when "WireCellsAll"

name

when "WireCells"

"WireCellsAll"

when "WireDebug"

"WireDebugAll"

Thanks!

For WireDebug, there are no WireDebug tests, and they wouldn't make sense.
Unless we're going to make snapshots & unit tests for our debug menu?

…e & ci

El-Fitz requested review from netbe, samwyndham and caldrian February 11, 2025 13:12

El-Fitz self-assigned this Feb 11, 2025

El-Fitz force-pushed the feat/wire-cells-upload-preview-images branch from 0f8aaab to 903d410 Compare February 11, 2025 13:12

caldrian reviewed Feb 11, 2025

View reviewed changes

samwyndham reviewed Feb 12, 2025

View reviewed changes

WireCells/Sources/WireCellsUI/Components/LocalImagePreview.swift Show resolved Hide resolved

samwyndham requested changes Feb 12, 2025

View reviewed changes

El-Fitz requested review from samwyndham and caldrian February 12, 2025 15:29

netbe requested changes Feb 12, 2025

View reviewed changes

fastlane/framework.rb Show resolved Hide resolved

wire-ios-build-assets Outdated Show resolved Hide resolved

...ReferenceImages/UploadImagePreviewTests/testDynamicTypeVariantsEmptyState.accessibility5.png Outdated Show resolved Hide resolved

El-Fitz force-pushed the feat/wire-cells-upload-preview-images branch from 2536775 to becf032 Compare February 12, 2025 16:10

El-Fitz requested a review from netbe February 12, 2025 16:10

netbe requested changes Feb 12, 2025

View reviewed changes

...Resources/ReferenceImages/UploadVideoPreviewTests/testColorSchemeVariantsEmptyState.dark.png Outdated Show resolved Hide resolved

wire-ios-build-assets Outdated Show resolved Hide resolved

El-Fitz requested a review from netbe February 12, 2025 19:42

netbe mentioned this pull request Feb 12, 2025

feat: wire cells upload preview documents - WPB-15843 #2528

Open

5 tasks

netbe approved these changes Feb 12, 2025

View reviewed changes

El-Fitz force-pushed the feat/wire-cells-upload-preview-images branch from f95930d to 90d1cd7 Compare February 13, 2025 08:49

caldrian approved these changes Feb 13, 2025

View reviewed changes

caldrian reviewed Feb 14, 2025

View reviewed changes

El-Fitz added 6 commits February 14, 2025 10:32

chore: rename targets for cleaner imports

8bb9d32

feat: use palette colors and wireframe placeholder image

dc0cca6

chore: misc workflow update & fixes

eb2f5c0

ci: add missing CI requirements

7697d4b

fix: remove dead code

a34b498

fix: remove extra file

b1f0167

El-Fitz and others added 9 commits February 14, 2025 10:32

ci: add padding to test snapshots

98e8ebb

fix: update snapshots

7e1041c

not-worthy-of-a-prefix: revert git submodule

4b47506

fix: a few minor structural changes - WPB-15844 (#2540)

412137f

add testplan

e89c3eb

enable upcoming features

af42b0e

fix: misc fixes, changes, and improvements related to pydio sdk updat…

184c4ad

…e & ci

ci: add ui snapshot tests for duration formats

d43a51b

ci: fix frameworks.rb

ebd2bd3

El-Fitz force-pushed the feat/wire-cells-upload-preview-images branch from 5b82dee to ebd2bd3 Compare February 14, 2025 09:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: wire cells upload images & videos previews - WPB-15844 #2513

feat: wire cells upload images & videos previews - WPB-15844 #2513

El-Fitz commented Feb 11, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Feb 11, 2025 •

edited

Loading

caldrian left a comment

caldrian Feb 11, 2025

El-Fitz Feb 11, 2025

caldrian Feb 13, 2025

caldrian Feb 13, 2025

El-Fitz Feb 13, 2025

El-Fitz commented Feb 11, 2025

datadog-wireapp bot commented Feb 11, 2025 •

edited

Loading

samwyndham left a comment

samwyndham Feb 12, 2025

netbe left a comment

netbe left a comment

netbe left a comment

caldrian Feb 13, 2025

El-Fitz Feb 13, 2025 •

edited

Loading

caldrian Feb 13, 2025

caldrian Feb 14, 2025

El-Fitz Feb 14, 2025

caldrian Feb 14, 2025

El-Fitz Feb 14, 2025 •

edited

Loading

-        when "WireCellsAll"
-            name
+        when "WireCells"
+            "WireCellsAll"
+        when "WireDebug"
+            "WireDebugAll"

feat: wire cells upload images & videos previews - WPB-15844 #2513

Are you sure you want to change the base?

feat: wire cells upload images & videos previews - WPB-15844 #2513

Conversation

El-Fitz commented Feb 11, 2025 • edited by github-actions bot Loading

Issue

Testing

Checklist

UI accessibility checklist

github-actions bot commented Feb 11, 2025 • edited Loading

Test Results

caldrian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

El-Fitz commented Feb 11, 2025

datadog-wireapp bot commented Feb 11, 2025 • edited Loading

Datadog Report

samwyndham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netbe left a comment

Choose a reason for hiding this comment

netbe left a comment

Choose a reason for hiding this comment

netbe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

El-Fitz Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

El-Fitz Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

El-Fitz commented Feb 11, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Feb 11, 2025 •

edited

Loading

datadog-wireapp bot commented Feb 11, 2025 •

edited

Loading

El-Fitz Feb 13, 2025 •

edited

Loading

El-Fitz Feb 14, 2025 •

edited

Loading