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.

Sign up for GitHub

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

Jump to bottom

feat(feedback): form ui iteration and tests #4536

Merged
merged 21 commits into from
Dec 7, 2024
Merged

feat(feedback): form ui iteration and tests #4536

merged 21 commits into from
Dec 7, 2024

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Nov 15, 2024 •
edited
Loading

for #4271; #skip-changelog

  • Adds UI tests to validate the functionality of the widget button and form functionality
  • Iterates on the form UI to fix up some interaction and layout issues.

Base automatically changed from armcknight/feat(user-feedback)/form to main November 19, 2024 19:58
@armcknight armcknight marked this pull request as ready for review November 19, 2024 20:22
philipphofmann
philipphofmann reviewed Nov 20, 2024
View reviewed changes
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

CI is pretty much broken. I'm waiting it for it to be green so I can have another pass before I approve this.

Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift Outdated Show resolved Hide resolved
Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift Outdated Show resolved Hide resolved
Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift Outdated Show resolved Hide resolved
@armcknight armcknight requested a review from philprime as a code owner December 3, 2024 20:57
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 3, 2024 •
edited
Loading

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1240.35 ms 1253.40 ms 13.05 ms
Size 22.30 KiB 756.41 KiB 734.11 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8c50edb 1212.98 ms 1233.72 ms 20.74 ms
c4e9093 1219.04 ms 1236.00 ms 16.96 ms
b9a9ffd 1201.61 ms 1215.06 ms 13.45 ms
7bb0873 1226.18 ms 1247.30 ms 21.12 ms
c50d363 1215.10 ms 1231.82 ms 16.72 ms
dcec216 1238.94 ms 1261.06 ms 22.12 ms
6ae86f6 1231.90 ms 1255.90 ms 24.00 ms
e8c8a05 1218.71 ms 1234.50 ms 15.79 ms
da5c197 1240.84 ms 1247.22 ms 6.39 ms
89bc37d 1240.76 ms 1248.24 ms 7.48 ms

App size

Revision Plain With Sentry Diff
8c50edb 20.76 KiB 432.31 KiB 411.55 KiB
c4e9093 21.58 KiB 671.30 KiB 649.72 KiB
b9a9ffd 21.58 KiB 418.43 KiB 396.85 KiB
7bb0873 22.85 KiB 407.09 KiB 384.24 KiB
c50d363 20.76 KiB 432.68 KiB 411.92 KiB
dcec216 20.76 KiB 432.88 KiB 412.11 KiB
6ae86f6 21.58 KiB 625.82 KiB 604.23 KiB
e8c8a05 21.58 KiB 683.51 KiB 661.92 KiB
da5c197 21.58 KiB 706.97 KiB 685.39 KiB
89bc37d 20.76 KiB 401.53 KiB 380.77 KiB

Previous results on branch: armcknight/feat(user-feedback)/ui-tests

Startup times

Revision Plain With Sentry Diff
2c2f957 1240.52 ms 1258.52 ms 18.00 ms
9c641ad 1241.84 ms 1264.26 ms 22.42 ms

App size

Revision Plain With Sentry Diff
2c2f957 22.30 KiB 750.20 KiB 727.90 KiB
9c641ad 22.30 KiB 750.21 KiB 727.91 KiB

@codecov Codecov
Copy link

codecov bot commented Dec 3, 2024 •
edited
Loading

Codecov Report

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

Project coverage is 90.873%. Comparing base (be4734f) to head (a4f1323).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...grations/UserFeedback/SentryUserFeedbackForm.swift 0.000% 150 Missing ⚠️
...ations/UserFeedback/SentryUserFeedbackWidget.swift 0.000% 3 Missing ⚠️
...onfiguration/SentryUserFeedbackConfiguration.swift 0.000% 1 Missing ⚠️
...rFeedback/SentryUserFeedbackWidgetButtonView.swift 0.000% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4536       +/-   ##
=============================================
- Coverage   91.011%   90.873%   -0.139%     
=============================================
  Files          617       617               
  Lines        70936     71024       +88     
  Branches     25319     25315        -4     
=============================================
- Hits         64560     64542       -18     
- Misses        6284      6390      +106     
  Partials        92        92
Files with missing lines Coverage Δ
...guration/SentryUserFeedbackFormConfiguration.swift 0.000% <ø> (ø)
...ration/SentryUserFeedbackWidgetConfiguration.swift 0.000% <ø> (ø)
...onfiguration/SentryUserFeedbackConfiguration.swift 0.000% <0.000%> (ø)
...rFeedback/SentryUserFeedbackWidgetButtonView.swift 0.000% <0.000%> (ø)
...ations/UserFeedback/SentryUserFeedbackWidget.swift 0.000% <0.000%> (ø)
...grations/UserFeedback/SentryUserFeedbackForm.swift 0.000% <0.000%> (ø)

... and 17 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be4734f...a4f1323. Read the comment docs.

@armcknight
Copy link
Member Author

armcknight commented Dec 4, 2024 •
edited
Loading

The UI test failures are due to the on-screen keyboard covering the lower part of the form. It's currently undismissable; there are a couple options, which I've outlined here: #4271 (comment)

ETA: fixed in #4590

@armcknight armcknight mentioned this pull request Dec 4, 2024
Merged
@armcknight armcknight changed the title test(user-feedback): ui tests to display form, submit, cancel feat(feedback): form ui iteration and tests Dec 4, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 4, 2024

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against a4f1323

philipphofmann
philipphofmann approved these changes Dec 6, 2024
View reviewed changes
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM, with a few minor suggestions.

Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift Show resolved Hide resolved
Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift Show resolved Hide resolved
Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift Show resolved Hide resolved
Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift Show resolved Hide resolved
@armcknight armcknight merged commit eba61d3 into main Dec 7, 2024
66 of 68 checks passed
@armcknight armcknight deleted the armcknight/feat(user-feedback)/ui-tests branch December 7, 2024 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipphofmann philipphofmann philipphofmann approved these changes

@brustolin brustolin Awaiting requested review from brustolin brustolin is a code owner

@philprime philprime Awaiting requested review from philprime philprime is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@armcknight @philipphofmann @getsentry-bot