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

Feature: Feedback form attachments #21114

Merged
merged 68 commits into from
Aug 8, 2024
Merged

Conversation

nbradbury
Copy link
Contributor

@nbradbury nbradbury commented Aug 5, 2024

This is the feature branch for uploading feedback form attachments. It contains the following child PRs:

To test:

  1. Tap Help on the Me page and ensure the "Contact email" is correct (this for step 6)
  2. Return to the Me page and tap "Send feedback"
  3. Enter a message
  4. Add a couple of attachments
  5. Tap submit and verify it all works
  6. When you get the email about the ticket, go to https://a8c.zendesk.com/agent/tickets/$ticketNumber and verify the attachments were added

Caveats:

  • Unlike iOS, there is no preview of attachments yet - that's being worked on separately in this branch
  • Like iOS, there isn't a way to cancel the attachment uploading. I suspect we'll want to add this at some point.

@nbradbury nbradbury added the User Feedback Users need something added or fixed label Aug 5, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Aug 5, 2024

7 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.
⚠️ Class ZendeskUploadHelper is missing tests, but unit-tests-exemption label was set to ignore this.
⚠️ Class ProgressDialogState is missing tests, but unit-tests-exemption label was set to ignore this.
⚠️ Class FeedbackFormAttachment is missing tests, but unit-tests-exemption label was set to ignore this.
⚠️ Class FeedbackFormUtils is missing tests, but unit-tests-exemption label was set to ignore this.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Aug 5, 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
Versionpr21114-bd868ee
Commitbd868ee
Direct Downloadjetpack-prototype-build-pr21114-bd868ee.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Aug 5, 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
Versionpr21114-bd868ee
Commitbd868ee
Direct Downloadwordpress-prototype-build-pr21114-bd868ee.apk
Note: Google Login is not supported on these builds.

@nbradbury nbradbury marked this pull request as ready for review August 8, 2024 12:52
@dcalhoun dcalhoun self-requested a review August 8, 2024 13:37
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.

Nice work!

I tested the flow in both the Jetpack and WordPress mobile apps on a Samsung Galaxy S20. I tested the happy path and disrupting submissions with network conditions. I did not encounter any significant issues.

In the WordPress app, there is no contact email. I presume we want to allow users to submit anonymous feedback from this app. Is that accurate?

I left a couple of suggestions, but I do not consider them blockers. Also, feel free to seek additional Android code feedback if you'd like.

Copy link

sonarqubecloud bot commented Aug 8, 2024

@nbradbury
Copy link
Contributor Author

nbradbury commented Aug 8, 2024

@dcalhoun Thanks for the review!

I noted the message box is not auto-focused and does not enable shift key be focused...Should we consider implementing that here?

Agreed. I've added auto-focus but I'm not sure why sentence capitalization isn't working since I'm requesting it. I'll keep looking into that, but I may file a separate issue for it to avoid holding up this PR. Update: it turns out I was doing it right in bd868ee after all, so problem solved.

I encountered this error when attempt to attach an image when "browsing" files on a Samsung Galaxy S20

Very good catch! I was able to reproduce this when browsing on my Pixel 7a and fixed it in ddfb08b.

@nbradbury
Copy link
Contributor Author

In the WordPress app, there is no contact email. I presume we want to allow users to submit anonymous feedback from this app. Is that accurate?

That's accurate, but is subject to change.

@nbradbury nbradbury merged commit 27b43d3 into trunk Aug 8, 2024
20 checks passed
@nbradbury nbradbury deleted the feature/feedback-form-attachments branch August 8, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unit-tests-exemption User Feedback Users need something added or fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants