-
-
Notifications
You must be signed in to change notification settings - Fork 342
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 UI: Use Image Picker libraries from integrations #4524
base: feedback-ui
Are you sure you want to change the base?
Conversation
…tive-image-picker`
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3e4cdf5 | 462.35 ms | 474.96 ms | 12.61 ms |
6b1624f | 462.78 ms | 465.13 ms | 2.35 ms |
df05370 | 477.62 ms | 491.63 ms | 14.00 ms |
e5d5735 | 452.70 ms | 453.04 ms | 0.34 ms |
0325426 | 477.32 ms | 457.43 ms | -19.89 ms |
269c976 | 448.08 ms | 428.86 ms | -19.22 ms |
2646c98 | 429.98 ms | 421.63 ms | -8.35 ms |
9402883 | 448.53 ms | 468.73 ms | 20.20 ms |
894ebb0 | 497.45 ms | 545.04 ms | 47.60 ms |
8cb898b | 438.83 ms | 420.58 ms | -18.25 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3e4cdf5 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
6b1624f | 17.75 MiB | 20.12 MiB | 2.37 MiB |
df05370 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
e5d5735 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
0325426 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
269c976 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
2646c98 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
9402883 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
894ebb0 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
8cb898b | 17.75 MiB | 20.12 MiB | 2.37 MiB |
Previous results on branch: antonis/feedback-ui-imagepicker-integration
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
741efe5 | 431.08 ms | 446.57 ms | 15.48 ms |
7f1934e | 553.33 ms | 558.29 ms | 4.96 ms |
a6ce163 | 430.81 ms | 432.06 ms | 1.25 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
741efe5 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
7f1934e | 17.75 MiB | 20.12 MiB | 2.37 MiB |
a6ce163 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
Android (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2646c98+dirty | 415.13 ms | 438.41 ms | 23.28 ms |
9402883+dirty | 436.49 ms | 518.94 ms | 82.45 ms |
0325426+dirty | 418.89 ms | 485.00 ms | 66.11 ms |
894ebb0+dirty | 417.81 ms | 499.74 ms | 81.93 ms |
8cb898b+dirty | 393.33 ms | 416.20 ms | 22.87 ms |
3e4cdf5+dirty | 642.13 ms | 702.23 ms | 60.10 ms |
6b1624f+dirty | 382.17 ms | 441.00 ms | 58.83 ms |
0459aee+dirty | 424.10 ms | 466.63 ms | 42.53 ms |
269c976+dirty | 395.13 ms | 438.37 ms | 43.24 ms |
e5d5735+dirty | 377.37 ms | 430.04 ms | 52.67 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2646c98+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
9402883+dirty | 7.15 MiB | 8.39 MiB | 1.23 MiB |
0325426+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
894ebb0+dirty | 7.15 MiB | 8.39 MiB | 1.23 MiB |
8cb898b+dirty | 7.15 MiB | 8.39 MiB | 1.24 MiB |
3e4cdf5+dirty | 7.15 MiB | 8.39 MiB | 1.23 MiB |
6b1624f+dirty | 7.15 MiB | 8.39 MiB | 1.23 MiB |
0459aee+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
269c976+dirty | 7.15 MiB | 8.39 MiB | 1.23 MiB |
e5d5735+dirty | 7.15 MiB | 8.39 MiB | 1.23 MiB |
Previous results on branch: antonis/feedback-ui-imagepicker-integration
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
7f1934e+dirty | 388.20 ms | 456.96 ms | 68.76 ms |
741efe5+dirty | 384.24 ms | 432.59 ms | 48.35 ms |
a6ce163+dirty | 382.11 ms | 424.33 ms | 42.23 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
7f1934e+dirty | 7.15 MiB | 8.39 MiB | 1.23 MiB |
741efe5+dirty | 7.15 MiB | 8.39 MiB | 1.23 MiB |
a6ce163+dirty | 7.15 MiB | 8.39 MiB | 1.23 MiB |
iOS (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
df05370+dirty | 1209.92 ms | 1216.55 ms | 6.63 ms |
0325426+dirty | 1228.88 ms | 1229.92 ms | 1.04 ms |
e5d5735+dirty | 1222.02 ms | 1222.22 ms | 0.20 ms |
894ebb0+dirty | 1224.33 ms | 1214.45 ms | -9.89 ms |
9402883+dirty | 1219.65 ms | 1217.94 ms | -1.72 ms |
3e4cdf5+dirty | 1222.53 ms | 1224.42 ms | 1.89 ms |
8cb898b+dirty | 1221.40 ms | 1231.78 ms | 10.37 ms |
6b1624f+dirty | 1224.65 ms | 1225.65 ms | 1.00 ms |
0459aee+dirty | 1232.82 ms | 1231.19 ms | -1.63 ms |
2646c98+dirty | 1218.51 ms | 1218.92 ms | 0.41 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
df05370+dirty | 2.63 MiB | 3.71 MiB | 1.07 MiB |
0325426+dirty | 2.63 MiB | 3.69 MiB | 1.06 MiB |
e5d5735+dirty | 2.63 MiB | 3.69 MiB | 1.06 MiB |
894ebb0+dirty | 2.63 MiB | 3.71 MiB | 1.07 MiB |
9402883+dirty | 2.63 MiB | 3.71 MiB | 1.07 MiB |
3e4cdf5+dirty | 2.63 MiB | 3.69 MiB | 1.06 MiB |
8cb898b+dirty | 2.63 MiB | 3.71 MiB | 1.08 MiB |
6b1624f+dirty | 2.63 MiB | 3.71 MiB | 1.07 MiB |
0459aee+dirty | 2.63 MiB | 3.69 MiB | 1.06 MiB |
2646c98+dirty | 2.63 MiB | 3.69 MiB | 1.06 MiB |
Previous results on branch: antonis/feedback-ui-imagepicker-integration
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
741efe5+dirty | 1212.52 ms | 1226.22 ms | 13.70 ms |
a6ce163+dirty | 1210.25 ms | 1214.21 ms | 3.96 ms |
7f1934e+dirty | 1222.53 ms | 1203.98 ms | -18.55 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
741efe5+dirty | 2.63 MiB | 3.71 MiB | 1.08 MiB |
a6ce163+dirty | 2.63 MiB | 3.71 MiB | 1.08 MiB |
7f1934e+dirty | 2.63 MiB | 3.71 MiB | 1.08 MiB |
iOS (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
df05370+dirty | 1221.14 ms | 1216.60 ms | -4.55 ms |
0325426+dirty | 1210.17 ms | 1216.37 ms | 6.20 ms |
e5d5735+dirty | 1217.78 ms | 1221.80 ms | 4.02 ms |
894ebb0+dirty | 1210.94 ms | 1202.08 ms | -8.85 ms |
9402883+dirty | 1217.71 ms | 1213.02 ms | -4.69 ms |
3e4cdf5+dirty | 1213.36 ms | 1221.31 ms | 7.95 ms |
8cb898b+dirty | 1209.39 ms | 1207.57 ms | -1.82 ms |
6b1624f+dirty | 1224.12 ms | 1220.73 ms | -3.39 ms |
0459aee+dirty | 1233.67 ms | 1239.80 ms | 6.12 ms |
2646c98+dirty | 1239.94 ms | 1246.90 ms | 6.96 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
df05370+dirty | 3.19 MiB | 4.27 MiB | 1.09 MiB |
0325426+dirty | 3.19 MiB | 4.26 MiB | 1.07 MiB |
e5d5735+dirty | 3.19 MiB | 4.26 MiB | 1.07 MiB |
894ebb0+dirty | 3.19 MiB | 4.27 MiB | 1.09 MiB |
9402883+dirty | 3.19 MiB | 4.27 MiB | 1.09 MiB |
3e4cdf5+dirty | 3.19 MiB | 4.26 MiB | 1.07 MiB |
8cb898b+dirty | 3.19 MiB | 4.28 MiB | 1.09 MiB |
6b1624f+dirty | 3.19 MiB | 4.27 MiB | 1.09 MiB |
0459aee+dirty | 3.19 MiB | 4.26 MiB | 1.07 MiB |
2646c98+dirty | 3.19 MiB | 4.26 MiB | 1.07 MiB |
Previous results on branch: antonis/feedback-ui-imagepicker-integration
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
741efe5+dirty | 1213.69 ms | 1223.18 ms | 9.49 ms |
a6ce163+dirty | 1219.88 ms | 1216.60 ms | -3.28 ms |
7f1934e+dirty | 1224.98 ms | 1235.30 ms | 10.32 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
741efe5+dirty | 3.19 MiB | 4.27 MiB | 1.09 MiB |
a6ce163+dirty | 3.19 MiB | 4.27 MiB | 1.09 MiB |
7f1934e+dirty | 3.19 MiB | 4.27 MiB | 1.09 MiB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just have one nit about any types.
And one question about testing this in plain RN apps.
Otherwise looks great, super simple and clean API!
} | ||
|
||
interface ExpoImageLibraryOptions { | ||
mediaTypes?: any[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be string[]
or 'images'[]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point @krystofwoldrich 👍
I've tried with a string[]
but it resulted in a typescript issue since the type was an enumeration.
'images'[]
works like a charm :) Thank you for the tip 🙇
Updated with 6666cf6
} | ||
|
||
interface ReactNativeImageLibraryOptions { | ||
mediaType: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be photo
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above I resorted to any to have type compatibility with the enum. 'photo' works as expected :)
Updated with d1e5107
export async function getDataFromUri(uri: string): Promise<Uint8Array> { | ||
const response = await fetch(uri); | ||
const blob = await response.blob(); | ||
|
||
return new Promise((resolve, reject) => { | ||
const reader = new FileReader(); | ||
reader.onloadend = () => { | ||
if (reader.result instanceof ArrayBuffer) { | ||
resolve(new Uint8Array(reader.result)); | ||
} else { | ||
reject(new Error('Failed to read file as UInt8Array')); | ||
} | ||
}; | ||
reader.onerror = reject; | ||
reader.readAsArrayBuffer(blob); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks super clean and doesn't need native impl, I've not realized we can use that.
Have you tried it in a plain new app? I just want to make sure that all the features are available out of the box, sometimes APIs are available in our samples because a dependency auto-polyfills them.
Make sense, we can update the changelog before the merge and the docs after.
Let's mention this in the changelog and the docs that only react-native-image-picker v7 and expo-image-picker v16 are supported. |
📢 Type of change
📜 Description
Use
expo-image-picker
andreact-native-image-picker
Image Picker libraries passed in integration.Note:
react-native-image-picker
:7.2
expo-image-picker
:16.0
💡 Motivation and Context
See #4302 (comment)
💚 How did you test it?
Manual with React Native and Expo sample apps
📝 Checklist
sendDefaultPII
is enabled🔮 Next steps
#skip-changelog