Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feedback form: attachment pager #21121

Merged
merged 38 commits into from
Aug 9, 2024
Merged

Conversation

nbradbury
Copy link
Contributor

@nbradbury nbradbury commented Aug 8, 2024

Fixes #21124

This PR adds a reusable media pager to the feedback form which displays a preview of the attachments. I also fixed some issues determining the correct file extension for selected media that was causing upload to fail, as well as added better error handling for failed requests.

To test:

  • Go to the Me page and tap "Submit feedback"
  • Add several attachments (both images and videos) and verify they appear in the image pager
  • Click X to remove an attachment and verify it's removed from the pager

Note that I added a new dependency: io.coil-kt:coil-video. I try to avoid new dependencies, but since we already use Coil for images I felt okay with adding another Coil dependency (specifically for video thumbnails).

attach.mp4

@dangermattic
Copy link
Collaborator

dangermattic commented Aug 8, 2024

3 Warnings
⚠️ strings.xml files should only be updated on release branches, when the translations are downloaded by our automation.
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Aug 8, 2024

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr21121-df91b9a
Commitdf91b9a
Direct Downloadwordpress-prototype-build-pr21121-df91b9a.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Aug 8, 2024

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr21121-df91b9a
Commitdf91b9a
Direct Downloadjetpack-prototype-build-pr21121-df91b9a.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented Aug 8, 2024

Codecov Report

Attention: Patch coverage is 0% with 63 lines in your changes missing coverage. Please review.

Project coverage is 40.57%. Comparing base (3698bcd) to head (df91b9a).
Report is 24 commits behind head on trunk.

Files Patch % Lines
...roid/ui/main/feedbackform/FeedbackFormViewModel.kt 0.00% 45 Missing ⚠️
...va/org/wordpress/android/util/extensions/UriExt.kt 0.00% 11 Missing ⚠️
...android/ui/main/feedbackform/FeedbackFormScreen.kt 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #21121      +/-   ##
==========================================
+ Coverage   40.56%   40.57%   +0.01%     
==========================================
  Files        1535     1535              
  Lines       70529    70505      -24     
  Branches    11660    11659       -1     
==========================================
  Hits        28607    28607              
+ Misses      39337    39313      -24     
  Partials     2585     2585              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wpmobilebot
Copy link
Contributor

Found 1 violations:

The PR caused some dependency changes (expand to see details)

+\--- io.coil-kt:coil-video:2.4.0
+     +--- androidx.core:core-ktx:1.9.0 -> 1.13.0 (*)
+     +--- io.coil-kt:coil-base:2.4.0 (*)
+     \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21 -> 1.9.10 (*)

Please review and act accordingly

@nbradbury nbradbury changed the title Feedback form: image pager Feedback form: attachment pager Aug 9, 2024
@nbradbury nbradbury marked this pull request as ready for review August 9, 2024 09:49
Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

Works well for me overall. 🚀

I noticed that a preview thumbnail was not created when attaching via the "Browse..." option and selecting a video in Google Drive. In contract, a preview thumbnail was created when attaching a video from the default media browser.

Screenshot

Screenshot_20240809_134205_Jetpack Pre-Alpha

@nbradbury
Copy link
Contributor Author

I noticed that a preview thumbnail was not created when attaching via the "Browse..." option and selecting a video in Google Drive.

@dcalhoun This may be due to a bug I just found. When I selected "Browse" and chose a file from Google Drive, it failed to create the temp file used for the preview. This is fixed now.

Copy link

sonarqubecloud bot commented Aug 9, 2024

@nbradbury nbradbury merged commit 1290c89 into trunk Aug 9, 2024
20 checks passed
@nbradbury nbradbury deleted the issue/21105-feedback-image-pager branch August 9, 2024 20:23
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.

Feedback Form: Attachment preview
4 participants