-
Notifications
You must be signed in to change notification settings - Fork 3k
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: Use attachment gallery everywhere #31308
Merged
pecanoro
merged 58 commits into
Expensify:main
from
margelo:feat/use-attachment-gallery-everywhere
Dec 15, 2023
Merged
Changes from all commits
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
32c2a12
move ImageTransformer component
52b5d14
feat: extract ImageLightbox to separate component and use everywhere
dff2a96
fix: image not loading initially
2ef7f8b
fix: simplify
f72fdc9
Merge branch 'main' into feat/use-attachment-gallery-everywhere
1fd70aa
fix: restructure props and unify zoom scale
b8d29cc
simplify bounce
bbf43b8
move bounce variables to constants
cbb6ad7
further improve ImageLightbox component
a1953e8
fix: restructure ImageTransformer and ImageLightbox components
bcc8539
fix: move GestureHandlerRootView to AttachmentModal
479dbbf
fix: improve zoom range on Android
b24bb32
Merge branch 'main' into feat/use-attachment-gallery-everywhere
chrispader 70c2dd8
fix: update react-native-fast-image patches
chrispader 8bd7d70
update comments
chrispader a40c790
add comments for prop types
chrispader d3d65ae
Merge branch 'main' into feat/use-attachment-gallery-everywhere
e5c3203
start in loading state
eb7d9ba
Update src/components/Lightbox.js
chrispader e2fcb4d
simplify patch
3694860
fix: review comments
f202868
Merge branch 'main' into feat/use-attachment-gallery-everywhere
84e8467
simplify multigesture content wrapper
3fb8957
improve zoomRange props and remove custom zoom ranges
270a3b3
fix: initial page wrong in carousel
d9d50bd
add optimizations for lightbox (only render fallback when in carousel…
931651d
fix: initial loading problem
5a5fd27
pass isSingleItem
735f237
add dependency
7ce9bc1
Merge branch 'main' into feat/use-attachment-gallery-everywhere
0e2f27e
fix: initial index
7c5ba50
remove log
4912e0a
Merge branch '@chrispader/fix-status-bar-safe-area' into feat/use-att…
b241680
simplify lightbox
9251eea
Merge branch 'main' into feat/use-attachment-gallery-everywhere
f74fd79
fix: Lightbox
307d075
simplify MultiGestureCanvas
9eced46
fix and simplify
7ea7ea0
remove comments
c09355c
impl: paging and multiple concurrent lightboxes
7b21a82
add comment
9bb72d6
fix: lint
e63226b
Merge branch 'main' into feat/use-attachment-gallery-everywhere
a7fce89
Merge branch 'main' into feat/use-attachment-gallery-everywhere
1e4c074
Merge branch 'main' into feat/use-attachment-gallery-everywhere
chrispader aa39a23
fix: onLoadEnd not triggered in "Send attachments"
chrispader cf71e1a
Merge branch 'main' into feat/use-attachment-gallery-everywhere
163468f
improve imports and exports
chrispader 226b3d5
fix: more stuff
chrispader 5531728
set minimum double tap scale
chrispader 3ca5359
Update src/components/MultiGestureCanvas/propTypes.js
chrispader af087f6
Update src/components/MultiGestureCanvas/propTypes.js
chrispader 89f5634
Update src/components/MultiGestureCanvas/propTypes.js
chrispader 41bfc91
Update src/components/MultiGestureCanvas/propTypes.js
chrispader 5cc91a6
Update src/components/MultiGestureCanvas/propTypes.js
chrispader 27ab60c
Update src/components/MultiGestureCanvas/index.js
chrispader 31b6455
remove defaultZoomRange
chrispader db5e3d4
fix: import
chrispader File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
File renamed without changes.
62 changes: 62 additions & 0 deletions
62
patches/react-native-fast-image+8.6.3+002+bitmap-downsampling.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
diff --git a/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageViewWithUrl.java b/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageViewWithUrl.java | ||
index 1339f5c..9dfec0c 100644 | ||
--- a/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageViewWithUrl.java | ||
+++ b/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageViewWithUrl.java | ||
@@ -176,7 +176,8 @@ class FastImageViewWithUrl extends AppCompatImageView { | ||
.apply(FastImageViewConverter | ||
.getOptions(context, imageSource, mSource) | ||
.placeholder(mDefaultSource) // show until loaded | ||
- .fallback(mDefaultSource)); // null will not be treated as error | ||
+ .fallback(mDefaultSource)) | ||
+ .transform(new ResizeTransformation()); | ||
|
||
if (key != null) | ||
builder.listener(new FastImageRequestListener(key)); | ||
diff --git a/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/ResizeTransformation.java b/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/ResizeTransformation.java | ||
new file mode 100644 | ||
index 0000000..1daa227 | ||
--- /dev/null | ||
+++ b/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/ResizeTransformation.java | ||
@@ -0,0 +1,41 @@ | ||
+package com.dylanvann.fastimage; | ||
+ | ||
+ import android.content.Context; | ||
+ import android.graphics.Bitmap; | ||
+ | ||
+ import androidx.annotation.NonNull; | ||
+ | ||
+ import com.bumptech.glide.load.Transformation; | ||
+ import com.bumptech.glide.load.engine.Resource; | ||
+ import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool; | ||
+ import com.bumptech.glide.load.resource.bitmap.BitmapResource; | ||
+ | ||
+ import java.security.MessageDigest; | ||
+ | ||
+ public class ResizeTransformation implements Transformation<Bitmap> { | ||
+ | ||
+ private final double MAX_BYTES = 25000000.0; | ||
+ | ||
+ @NonNull | ||
+ @Override | ||
+ public Resource<Bitmap> transform(@NonNull Context context, @NonNull Resource<Bitmap> resource, int outWidth, int outHeight) { | ||
+ Bitmap toTransform = resource.get(); | ||
+ | ||
+ if (toTransform.getByteCount() > MAX_BYTES) { | ||
+ double scaleFactor = Math.sqrt(MAX_BYTES / (double) toTransform.getByteCount()); | ||
+ int newHeight = (int) (outHeight * scaleFactor); | ||
+ int newWidth = (int) (outWidth * scaleFactor); | ||
+ | ||
+ BitmapPool pool = GlideApp.get(context).getBitmapPool(); | ||
+ Bitmap scaledBitmap = Bitmap.createScaledBitmap(toTransform, newWidth, newHeight, true); | ||
+ return BitmapResource.obtain(scaledBitmap, pool); | ||
+ } | ||
+ | ||
+ return resource; | ||
+ } | ||
+ | ||
+ @Override | ||
+ public void updateDiskCacheKey(@NonNull MessageDigest messageDigest) { | ||
+ messageDigest.update(("ResizeTransformation").getBytes()); | ||
+ } | ||
+ } | ||
\ No newline at end of file |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Just an initial idea, not a blocker. Perhaps in the future, this value can be adjusted based on the system properties, similar to the values in canvas exception. This way, new devices might be able to see clearer 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.
Yes agree. Definitely something for a future PR. I think @ArekChr tested this value very thoroughly, so it should be high enough to display high-resolution images on high-end displays as well.
Could we already create an (internal) issue for this, so i can jump onto this at some point in the future? @pecanoro @dylanexpensify