Add textScale(r) value to Flutter context #1886

denrase · 2024-02-20T14:38:08Z

📜 Description

Add Flutter text scale to event contexts.

💡 Motivation and Context

Closes #1832

💚 How did you test it?

Tests

📝 Checklist

I reviewed submitted code
I added tests to verify changes
No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
I updated the docs if needed
All tests passing
No breaking changes

🔮 Next steps

Handle deprecation

github-actions · 2024-02-20T14:39:16Z

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

Generated by 🚫 dangerJS against 4e2ed6b

github-actions · 2024-02-20T14:50:50Z

Android Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	374.60 ms	439.78 ms	65.18 ms
Size	6.34 MiB	7.29 MiB	970.36 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`1ac008b`	370.04 ms	435.58 ms	65.54 ms
`11fb408`	320.10 ms	380.24 ms	60.14 ms
`64af39c`	386.80 ms	471.11 ms	84.31 ms
`2261c15`	370.00 ms	455.88 ms	85.88 ms
`86d4841`	286.35 ms	372.43 ms	86.08 ms
`6078ddc`	385.72 ms	463.61 ms	77.89 ms
`280ab9f`	316.46 ms	372.98 ms	56.52 ms
`cdf7172`	348.54 ms	390.81 ms	42.27 ms
`e3d9076`	388.08 ms	488.51 ms	100.43 ms
`fcd1ee4`	298.96 ms	376.04 ms	77.09 ms

App size

Revision	Plain	With Sentry	Diff
`1ac008b`	6.33 MiB	7.27 MiB	954.13 KiB
`11fb408`	6.06 MiB	7.10 MiB	1.04 MiB
`64af39c`	6.27 MiB	7.20 MiB	958.83 KiB
`2261c15`	6.27 MiB	7.20 MiB	957.75 KiB
`86d4841`	6.15 MiB	7.13 MiB	1000.49 KiB
`6078ddc`	6.34 MiB	7.29 MiB	967.80 KiB
`280ab9f`	6.16 MiB	7.14 MiB	1010.85 KiB
`cdf7172`	5.94 MiB	6.95 MiB	1.01 MiB
`e3d9076`	6.34 MiB	7.28 MiB	967.79 KiB
`fcd1ee4`	6.16 MiB	7.13 MiB	1000.85 KiB

Previous results on branch: feat/text-scale-value

Startup times

Revision	Plain	With Sentry	Diff
`b898149`	371.40 ms	429.18 ms	57.78 ms
`09c7edb`	364.33 ms	417.41 ms	53.09 ms
`ed83106`	358.92 ms	428.38 ms	69.46 ms
`58a6940`	379.59 ms	443.50 ms	63.91 ms
`4e1cc5a`	370.63 ms	446.77 ms	76.15 ms
`a2d3c75`	355.08 ms	424.69 ms	69.61 ms
`f6f14d5`	392.37 ms	476.21 ms	83.84 ms
`4468fb8`	378.87 ms	409.94 ms	31.06 ms
`1e48b4b`	384.52 ms	458.55 ms	74.03 ms

App size

Revision	Plain	With Sentry	Diff
`b898149`	6.34 MiB	7.29 MiB	968.57 KiB
`09c7edb`	6.34 MiB	7.29 MiB	968.55 KiB
`ed83106`	6.34 MiB	7.29 MiB	969.65 KiB
`58a6940`	6.34 MiB	7.29 MiB	968.57 KiB
`4e1cc5a`	6.34 MiB	7.29 MiB	968.55 KiB
`a2d3c75`	6.33 MiB	7.27 MiB	954.26 KiB
`f6f14d5`	6.34 MiB	7.28 MiB	965.48 KiB
`4468fb8`	6.34 MiB	7.28 MiB	965.55 KiB
`1e48b4b`	6.33 MiB	7.27 MiB	955.21 KiB

github-actions · 2024-02-20T14:56:28Z

iOS Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1366.33 ms	1417.37 ms	51.03 ms
Size	8.33 MiB	9.40 MiB	1.07 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`aa950e9`	1275.17 ms	1295.33 ms	20.16 ms
`1ac008b`	1223.96 ms	1247.77 ms	23.81 ms
`5603ab2`	1268.47 ms	1280.73 ms	12.26 ms
`4c78360`	1230.35 ms	1252.37 ms	22.01 ms
`8a10ab7`	1226.49 ms	1250.52 ms	24.03 ms
`1e094d3`	1237.54 ms	1243.51 ms	5.97 ms
`74e1fdd`	1240.08 ms	1258.28 ms	18.19 ms
`fbf42af`	1253.76 ms	1269.51 ms	15.76 ms
`f275487`	1291.65 ms	1339.92 ms	48.26 ms
`2331d89`	1260.86 ms	1281.24 ms	20.39 ms

App size

Revision	Plain	With Sentry	Diff
`aa950e9`	8.16 MiB	9.17 MiB	1.01 MiB
`1ac008b`	8.32 MiB	9.38 MiB	1.06 MiB
`5603ab2`	8.15 MiB	9.12 MiB	990.57 KiB
`4c78360`	8.32 MiB	9.38 MiB	1.06 MiB
`8a10ab7`	8.28 MiB	9.34 MiB	1.06 MiB
`1e094d3`	8.29 MiB	9.37 MiB	1.08 MiB
`74e1fdd`	8.32 MiB	9.38 MiB	1.06 MiB
`fbf42af`	8.16 MiB	9.17 MiB	1.01 MiB
`f275487`	8.32 MiB	9.38 MiB	1.05 MiB
`2331d89`	8.16 MiB	9.17 MiB	1.01 MiB

Previous results on branch: feat/text-scale-value

Startup times

Revision	Plain	With Sentry	Diff
`f6f14d5`	1264.23 ms	1279.53 ms	15.30 ms
`4468fb8`	1245.40 ms	1270.60 ms	25.20 ms
`b898149`	1254.63 ms	1278.27 ms	23.64 ms
`4e1cc5a`	1251.25 ms	1267.67 ms	16.42 ms
`1e48b4b`	1245.10 ms	1253.37 ms	8.27 ms
`58a6940`	1250.35 ms	1276.40 ms	26.05 ms
`a2d3c75`	1250.11 ms	1275.98 ms	25.87 ms
`ed83106`	1235.14 ms	1246.45 ms	11.31 ms
`09c7edb`	1246.25 ms	1270.21 ms	23.96 ms

App size

Revision	Plain	With Sentry	Diff
`f6f14d5`	8.33 MiB	9.40 MiB	1.07 MiB
`4468fb8`	8.33 MiB	9.40 MiB	1.07 MiB
`b898149`	8.33 MiB	9.40 MiB	1.07 MiB
`4e1cc5a`	8.33 MiB	9.40 MiB	1.07 MiB
`1e48b4b`	8.32 MiB	9.39 MiB	1.06 MiB
`58a6940`	8.33 MiB	9.40 MiB	1.07 MiB
`a2d3c75`	8.32 MiB	9.38 MiB	1.06 MiB
`ed83106`	8.33 MiB	9.40 MiB	1.07 MiB
`09c7edb`	8.33 MiB	9.40 MiB	1.07 MiB

buenaflor · 2024-02-21T10:36:26Z

Blocked by: #1881, might need to be addressed first before we merge this

kuhnroyal · 2024-03-04T23:39:12Z

flutter/lib/src/event_processor/widget_event_processor.dart

+      return event;
+    }
+    final textScale =
+        textScaleFactor == 1.0 ? 'no scaling' : 'linear (${textScaleFactor}x)';


You can just use textScaler?.toString() ?? 'no-scaling' here and avoid the deprecation.

Looks like toString is only overridden in _LinearTextScaler class, while _ClampedTextScaler does not override toString.

Right... but since you can't figure out if it is clamped anyway, because of private classes, maybe just go with textScaler?.toString() ?? 'unknown'?

Also I don't know in which Flutter version TextScaler was introduced. Need to be careful about not breaking backwards compatibility.

Don't really know that the output of toString will be when it's non-null. Opted to use the scale(fontSize) function for now with value 1. Does this add enough context so that it's useful? Don't really know how else we should handle non-liniear scalers.

@kuhnroyal Any additional thoughts?

Also I don't know in which Flutter version TextScaler was introduced. Need to be careful about not breaking backwards compatibility.

Still wondering about this and have not had the time to look into it yet.

The TextScaler was only added in Flutter 3.16 - see https://docs.flutter.dev/release/breaking-changes/deprecate-textscalefactor?
So there is probably a switch needed, or for now just live with the deprecation and use the textScaleFactor.

@denrase can we use the dynamic approach here that we do for other code that needs backwards compatibility?

If we only have this static method to access the text scaler, we cannot use the dynamic approach. In that case we can only use the deprecated method available on all flutter versions we have to support.

# Conflicts: # flutter/example/lib/main.dart # flutter/lib/src/sentry_flutter.dart

kuhnroyal

I think this is good. The GlobalKey will lead to errors when someone tries to use multiple SentryWidgets - is this something to worry about?

denrase · 2024-03-05T13:00:14Z

@kuhnroyal TheSentryWidget is not supposed to be used multiple times, so this should not be an issue as long as it is used correctly.

buenaflor · 2024-03-05T13:19:17Z

@denrase we should prolly document that somewhere (if not already)

kuhnroyal

I think this is the best possible way atm.

…art into feat/text-scale-value

denrase added 2 commits February 20, 2024 14:56

entrich event contexts app with text scale

a2d3c75

read textScale from sentry widget event processor

7da4423

denrase added 4 commits February 20, 2024 15:39

revert change

50bd207

revert change

1e48b4b

update changelog

0fcf836

Merge branch 'main' into feat/text-scale-value

48881f2

denrase marked this pull request as ready for review February 20, 2024 14:42

denrase requested review from krystofwoldrich, stefanosiano and buenaflor as code owners February 20, 2024 14:42

buenaflor added the Blocked label Mar 4, 2024

kuhnroyal reviewed Mar 4, 2024

View reviewed changes

denrase added 3 commits March 5, 2024 12:55

Merge branch 'main' into feat/text-scale-value

d3c0f35

# Conflicts: # flutter/example/lib/main.dart # flutter/lib/src/sentry_flutter.dart

update changelog

f6f14d5

Use scale method instead of deprecated textScaleFactor

4468fb8

denrase requested a review from kuhnroyal March 5, 2024 12:27

kuhnroyal approved these changes Mar 5, 2024

View reviewed changes

denrase added 7 commits March 12, 2024 11:20

Merge branch 'main' into feat/text-scale-value

f49cced

fix changelog

58a6940

Merge branch 'main' into feat/text-scale-value

c3852ee

fix changelog

b898149

Merge branch 'main' into feat/text-scale-value

99cc06a

use maybeTextScaleFactorOf method

f049f6b

fix changelog

09c7edb

denrase requested a review from kuhnroyal March 19, 2024 13:19

kuhnroyal approved these changes Mar 19, 2024

View reviewed changes

denrase added 8 commits March 25, 2024 15:13

Merge branch 'main' into feat/text-scale-value

0fd568b

fix changelog

4e1cc5a

Merge branch 'main' into feat/text-scale-value

62ccb79

use textScaleFactorOf

17b4845

Merge branch 'feat/text-scale-value' of github.com:getsentry/sentry-d…

9e67403

…art into feat/text-scale-value

fix tests

d0d6baa

update doc

630bfad

Merge branch 'main' into feat/text-scale-value

21fee1c

buenaflor approved these changes Apr 10, 2024

View reviewed changes

buenaflor added 3 commits April 10, 2024 09:48

Merge branch 'main' into feat/text-scale-value

ed83106

Update CHANGELOG.md

27ede51

Merge branch 'main' into feat/text-scale-value

4e2ed6b

buenaflor merged commit 6034b0a into main Apr 10, 2024
123 checks passed

buenaflor deleted the feat/text-scale-value branch April 10, 2024 10:23

buenaflor removed the Blocked label Apr 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add textScale(r) value to Flutter context #1886

Add textScale(r) value to Flutter context #1886

denrase commented Feb 20, 2024 •

edited

github-actions bot commented Feb 20, 2024 •

edited

github-actions bot commented Feb 20, 2024 •

edited

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/text-scale-value

Startup times

App size

github-actions bot commented Feb 20, 2024 •

edited

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/text-scale-value

Startup times

App size

buenaflor commented Feb 21, 2024 •

edited

kuhnroyal Mar 4, 2024 •

edited

denrase Mar 5, 2024

kuhnroyal Mar 5, 2024

kuhnroyal Mar 5, 2024

denrase Mar 5, 2024

denrase Mar 12, 2024

kuhnroyal Mar 12, 2024

kuhnroyal Mar 12, 2024

buenaflor Mar 12, 2024

denrase Mar 19, 2024

kuhnroyal left a comment

denrase commented Mar 5, 2024

buenaflor commented Mar 5, 2024

kuhnroyal left a comment

Add textScale(r) value to Flutter context #1886

Add textScale(r) value to Flutter context #1886

Conversation

denrase commented Feb 20, 2024 • edited

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Feb 20, 2024 • edited

github-actions bot commented Feb 20, 2024 • edited

Android Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/text-scale-value

Startup times

App size

github-actions bot commented Feb 20, 2024 • edited

iOS Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/text-scale-value

Startup times

App size

buenaflor commented Feb 21, 2024 • edited

kuhnroyal Mar 4, 2024 • edited

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

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

kuhnroyal left a comment

Choose a reason for hiding this comment

denrase commented Mar 5, 2024

buenaflor commented Mar 5, 2024

kuhnroyal left a comment

Choose a reason for hiding this comment

denrase commented Feb 20, 2024 •

edited

github-actions bot commented Feb 20, 2024 •

edited

github-actions bot commented Feb 20, 2024 •

edited

github-actions bot commented Feb 20, 2024 •

edited

buenaflor commented Feb 21, 2024 •

edited

kuhnroyal Mar 4, 2024 •

edited