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

fix: Don't redact clipped views #4325

Merged
merged 5 commits into from
Sep 13, 2024

Conversation

brustolin
Copy link
Contributor

📜 Description

A label that is outside of bounds of a view with clipBounds true is no longer being redacted.

Screen Before After
image 747730062 107031 copy 747729969 339359

💚 How did you test it?

Unit tests

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Copy link

codecov bot commented Sep 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.670%. Comparing base (35bb1e7) to head (fb56b85).
Report is 1 commits behind head on fix/redact-under-translucent.

Additional details and impacted files

Impacted file tree graph

@@                      Coverage Diff                       @@
##           fix/redact-under-translucent     #4325   +/-   ##
==============================================================
  Coverage                        91.669%   91.670%           
==============================================================
  Files                               618       618           
  Lines                             50286     50325   +39     
  Branches                          18123     18135   +12     
==============================================================
+ Hits                              46097     46133   +36     
- Misses                             4097      4099    +2     
- Partials                             92        93    +1     
Files with missing lines Coverage Δ
Sources/Swift/Tools/SentryViewPhotographer.swift 100.000% <100.000%> (ø)
Sources/Swift/Tools/UIRedactBuilder.swift 100.000% <100.000%> (ø)
...ests/SentryTests/SentryViewPhotographerTests.swift 99.193% <100.000%> (+0.193%) ⬆️
Tests/SentryTests/UIRedactBuilderTests.swift 100.000% <100.000%> (ø)

... and 5 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 35bb1e7...fb56b85. Read the comment docs.

Copy link

github-actions bot commented Sep 11, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1225.31 ms 1246.02 ms 20.71 ms
Size 21.58 KiB 708.51 KiB 686.92 KiB

Baseline results on branch: fix/redact-under-translucent

Startup times

Revision Plain With Sentry Diff
a268d0b 1223.94 ms 1242.94 ms 19.00 ms
70ebda6 1238.09 ms 1247.11 ms 9.03 ms
b2f8339 1218.12 ms 1243.32 ms 25.20 ms
be34bdf 1228.30 ms 1244.78 ms 16.48 ms
11b37b6 1239.22 ms 1259.82 ms 20.59 ms
bf4bb59 1231.86 ms 1251.20 ms 19.35 ms
fda51b1 1229.89 ms 1237.02 ms 7.13 ms
1e7fd29 1242.35 ms 1260.59 ms 18.24 ms
8ee29d0 1227.21 ms 1242.84 ms 15.63 ms

App size

Revision Plain With Sentry Diff
a268d0b 21.58 KiB 707.29 KiB 685.70 KiB
70ebda6 21.58 KiB 707.13 KiB 685.55 KiB
b2f8339 21.58 KiB 707.34 KiB 685.76 KiB
be34bdf 21.58 KiB 707.33 KiB 685.75 KiB
11b37b6 21.58 KiB 707.84 KiB 686.26 KiB
bf4bb59 21.58 KiB 707.34 KiB 685.76 KiB
fda51b1 21.58 KiB 707.34 KiB 685.76 KiB
1e7fd29 21.58 KiB 707.14 KiB 685.56 KiB
8ee29d0 21.58 KiB 707.40 KiB 685.82 KiB

Previous results on branch: fix/out-of-clip-redact

Startup times

Revision Plain With Sentry Diff
242e2a8 1231.12 ms 1249.19 ms 18.07 ms

App size

Revision Plain With Sentry Diff
242e2a8 21.58 KiB 707.91 KiB 686.33 KiB

Copy link
Member

@armcknight armcknight left a comment

Choose a reason for hiding this comment

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

Thanks for the extra explanation in the comments 🙏🏻 Seems fine

@brustolin brustolin merged commit cf60b5e into fix/redact-under-translucent Sep 13, 2024
62 of 63 checks passed
@brustolin brustolin deleted the fix/out-of-clip-redact branch September 13, 2024 13:56
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